code review 7124044: go.net/publicsuffix: add an EffectiveTLDPlus1 function.

go.net/publicsuffix: add an EffectiveTLDPlus1 function.

Also expand the gen.go subset to cover the test cases from
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/data/test_psl.txt

[nigeltao, 2013-01-16 12:32:44]

Hello dr.volker.dobler@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.net

[volker.dobler, 2013-01-17 07:53:53]

Nothing really wrong if EffectiveTLDPlus1 wouldn't be
exported.

https://codereview.appspot.com/7124044/diff/5001/publicsuffix/list.go
File publicsuffix/list.go (right):

https://codereview.appspot.com/7124044/diff/5001/publicsuffix/list.go#newcode123
publicsuffix/list.go:123: func EffectiveTLDPlus1(domain string) (string, error)
{
This function returns two erros of different quality:
The "cannot derive eTLD+1" which just indicates that
domain is too short and the "invalid public suffix" case
which indicates a broken implementation of PublicSuffix().

The "too short domain"-case will happen during construction
of a stoarge key if a host cookie is set e.g. on co.uk
(which is legit). But the "broken implementation"-case
(hopefully) won't happen at all.

Currently the two error case cannot be differentiated
properly. I see two fixes:
a) Two different error types which would allow to
   type switch.
b) Panic if PublicSuffix is broken.

I am not sure, but currently the "invalid public suffic"
case cannot happen at all, or? I tried to come up with an
input to PublicSuffix which would trigger this case
but couldn't find one.  Did I miss something obvious?

Regarding its use in package cookiejar: Cookiejar will
need its own EffectiveTLDPlus1 anyway as cookjar works
with pluggable PublicSuffixList implementations.  So
we can differentiate the two error cases there.

Conclusion:
 - Fine as it allows to test the "official" testcases.
 - Fine as it exports useful functionality to the user.
 - Package cookiejar will reimplement it anyway with
   finer error handling.

[pmylund, 2013-01-17 08:58:10]

Just wanted to note that I'm using EffectiveTLDPlusOne from cookiejar in several
production applications that don't deal with cookies, and find it immensely
useful. There are many other cases where you'd want to "get root domain" of an
FQDN X. Please do not unexport, especially in "publicsuffix" which is more
general.

As an aside, I would suggest the name "EffectiveTLDPlusOne" instead of
"EffectiveTLDPlus1".

[nigeltao, 2013-01-21 11:41:22]

EffectiveTLDPlus1 was renamed to EffectiveTLDPlusOne.

Please take another look.

https://codereview.appspot.com/7124044/diff/5001/publicsuffix/list.go
File publicsuffix/list.go (right):

https://codereview.appspot.com/7124044/diff/5001/publicsuffix/list.go#newcode123
publicsuffix/list.go:123: func EffectiveTLDPlus1(domain string) (string, error)
{
The "invalid public suffix" case shouldn't happen, but panicking seems
inappropriate, and returning a non-nil error seems preferable to returning a
bogus answer and nil error.

As for differentiating the two cases, I don't see the pressing need. If it turns
out there is one, then we can always add FooError and BarError types later. The
reverse isn't true; backwards compatability prevents ever taking FooError types
away. Yes, the go.net sub-repository isn't subject to strict back-compat
guarantees, but it's still better to avoid breaking code if you can.

This function isn't for use with package cookiejar per se. As Patrick said, for
better or worse, there are non-cookiejar uses for calculating the eTLD+1. For
example, a browsers's URL bar may wish to show different parts of the host in
different colors.

[volker.dobler, 2013-01-21 16:55:26]

LGTM

[nigeltao, 2013-01-22 10:23:46]

*** Submitted as
https://code.google.com/p/go/source/detail?r=f0a629bb85a7&repo=net ***

go.net/publicsuffix: add an EffectiveTLDPlus1 function.

Also expand the gen.go subset to cover the test cases from
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/data/test_psl...

R=dr.volker.dobler, patrick
CC=golang-dev
https://codereview.appspot.com/7124044