code review 13638045: net/http: bump Go version in default User-Agent to 1.2.

net/http: bump Go version in default User-Agent to 1.2.

[dsymonds, 2013-09-11 22:48:54]

Hello golang-dev@googlegroups.com,

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

[adg, 2013-09-11 23:20:53]

Use the defaultUserAgent constant in the tests?


On 12 September 2013 08:48, <dsymonds@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> net/http: bump Go version in default User-Agent to 1.2.
>
> Please review this at
https://codereview.appspot.**com/13638045/<https://codereview.appspot.com/136...
>
> Affected files (+20, -20 lines):
>   M src/pkg/net/http/request.go
>   M src/pkg/net/http/requestwrite_**test.go
>
>
> Index: src/pkg/net/http/request.go
> ==============================**==============================**=======
> --- a/src/pkg/net/http/request.go
> +++ b/src/pkg/net/http/request.go
> @@ -291,7 +291,7 @@
>         return def
>  }
>
> -const defaultUserAgent = "Go 1.1 package http"
> +const defaultUserAgent = "Go 1.2 package http"
>
>  // Write writes an HTTP/1.1 request -- header and body -- in wire format.
>  // This method consults the following fields of the request:
> Index: src/pkg/net/http/requestwrite_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/net/http/**requestwrite_test.go
> +++ b/src/pkg/net/http/**requestwrite_test.go
> @@ -93,13 +93,13 @@
>
>                 WantWrite: "GET /search HTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Transfer-Encoding: chunked\r\n\r\n" +
>                         chunk("abcdef") + chunk(""),
>
>                 WantProxy: "GET http://www.google.com/searchHTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Transfer-Encoding: chunked\r\n\r\n" +
>                         chunk("abcdef") + chunk(""),
>         },
> @@ -123,14 +123,14 @@
>
>                 WantWrite: "POST /search HTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Connection: close\r\n" +
>                         "Transfer-Encoding: chunked\r\n\r\n" +
>                         chunk("abcdef") + chunk(""),
>
>                 WantProxy: "POST http://www.google.com/searchHTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Connection: close\r\n" +
>                         "Transfer-Encoding: chunked\r\n\r\n" +
>                         chunk("abcdef") + chunk(""),
> @@ -156,7 +156,7 @@
>
>                 WantWrite: "POST /search HTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Connection: close\r\n" +
>                         "Content-Length: 6\r\n" +
>                         "\r\n" +
> @@ -164,7 +164,7 @@
>
>                 WantProxy: "POST http://www.google.com/searchHTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Connection: close\r\n" +
>                         "Content-Length: 6\r\n" +
>                         "\r\n" +
> @@ -187,14 +187,14 @@
>
>                 WantWrite: "POST / HTTP/1.1\r\n" +
>                         "Host: example.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Content-Length: 6\r\n" +
>                         "\r\n" +
>                         "abcdef",
>
>                 WantProxy: "POST http://example.com/ HTTP/1.1\r\n" +
>                         "Host: example.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Content-Length: 6\r\n" +
>                         "\r\n" +
>                         "abcdef",
> @@ -210,7 +210,7 @@
>
>                 WantWrite: "GET /search HTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "\r\n",
>         },
>
> @@ -232,13 +232,13 @@
>                 // Also, nginx expects it for POST and PUT.
>                 WantWrite: "POST / HTTP/1.1\r\n" +
>                         "Host: example.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Content-Length: 0\r\n" +
>                         "\r\n",
>
>                 WantProxy: "POST / HTTP/1.1\r\n" +
>                         "Host: example.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Content-Length: 0\r\n" +
>                         "\r\n",
>         },
> @@ -258,13 +258,13 @@
>
>                 WantWrite: "POST / HTTP/1.1\r\n" +
>                         "Host: example.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Transfer-Encoding: chunked\r\n\r\n" +
>                         chunk("x") + chunk(""),
>
>                 WantProxy: "POST / HTTP/1.1\r\n" +
>                         "Host: example.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "Transfer-Encoding: chunked\r\n\r\n" +
>                         chunk("x") + chunk(""),
>         },
> @@ -325,7 +325,7 @@
>
>                 WantWrite: "GET /foo HTTP/1.1\r\n" +
>                         "Host: \r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "X-Foo: X-Bar\r\n\r\n",
>         },
>
> @@ -351,7 +351,7 @@
>
>                 WantWrite: "GET /search HTTP/1.1\r\n" +
>                         "Host: \r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n\r\n",
> +                       "User-Agent: Go 1.2 package http\r\n\r\n",
>         },
>
>         // Opaque test #1 from golang.org/issue/4860
> @@ -370,7 +370,7 @@
>
>                 WantWrite: "GET /%2F/%2F/ HTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n\r\n",
> +                       "User-Agent: Go 1.2 package http\r\n\r\n",
>         },
>
>         // Opaque test #2 from golang.org/issue/4860
> @@ -389,7 +389,7 @@
>
>                 WantWrite: "GET http://y.google.com/%2F/%2F/HTTP/1.1\r\n" +
>                         "Host: x.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n\r\n",
> +                       "User-Agent: Go 1.2 package http\r\n\r\n",
>         },
>
>         // Testing custom case in header keys. Issue 5022.
> @@ -411,7 +411,7 @@
>
>                 WantWrite: "GET / HTTP/1.1\r\n" +
>                         "Host: www.google.com\r\n" +
> -                       "User-Agent: Go 1.1 package http\r\n" +
> +                       "User-Agent: Go 1.2 package http\r\n" +
>                         "ALL-CAPS: x\r\n" +
>                         "\r\n",
>         },
> @@ -498,7 +498,7 @@
>         }
>         expected := "POST / HTTP/1.1\r\n" +
>                 "Host: foo.com\r\n" +
> -               "User-Agent: Go 1.1 package http\r\n" +
> +               "User-Agent: Go 1.2 package http\r\n" +
>                 "Transfer-Encoding: chunked\r\n\r\n" +
>                 // TODO: currently we don't buffer before chunking, so we
> get a
>                 // single "m" chunk before the other chunks, as this was
> the 1-byte
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>

[minux1, 2013-09-11 23:26:03]

i understand why we add 1.1 to the UA, but do we really need to embed the
version number in it?

for the sake of avoiding being detected as virus/torjan, i don't think it
is better than not embedding it.

we really should tell the antivirus vendor about this issue, instead of
tweaking UA
before each release.

[adg, 2013-09-11 23:28:19]

Is this really an issue for AV vendors? I must have missed the last time it
came up.

I'd be in favor of removing the version entirely. I'm not sure why the HTTP
server should know how the client program was built.

[dave_cheney.net, 2013-09-11 23:34:30]

+1 to removing the version. 

On 12/09/2013, at 9:27, Andrew Gerrand <adg@golang.org> wrote:

> Is this really an issue for AV vendors? I must have missed the last time it
came up.
> 
> I'd be in favor of removing the version entirely. I'm not sure why the HTTP
server should know how the client program was built.
> -- 
>  
> --- 
> You received this message because you are subscribed to the Google Groups
"golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

[minux1, 2013-09-11 23:40:46]

On Sep 11, 2013 7:28 PM, "Andrew Gerrand" <adg@golang.org> wrote:
>
> Is this really an issue for AV vendors? I must have missed the last time
it came up.
https://codereview.appspot.com/7532043

it's an IDS issue. I don't know if the situation has changed.
> I'd be in favor of removing the version entirely. I'm not sure why the
HTTP server should know how the client program was built.

[rsc, 2013-09-11 23:45:33]

keep the version but leave it at 1.1.

russ

[rsc, 2013-09-11 23:46:34]

that is, do nothing.

[dsymonds, 2013-09-11 23:53:37]

The version is useful because HTTP client behaviour changes over time.
A go 1.1 HTTP client behaves differently than a go 1.2 HTTP client. An
HTTP server in theory should not care, but if, for instance, the go
1.1 HTTP client is badly behaved then a server could block requests
from just that version and not all go HTTP clients.

[dsymonds, 2013-09-11 23:58:04]

Hello golang-dev@googlegroups.com, adg@golang.org, minux.ma@gmail.com,
dave@cheney.net, rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2013-09-11 23:59:28]

That is not the precedent I was trying to set with this. I was only working
around dumb antivirus software I don't believe we should be advertising the
version of the client, any more than we should be sending

    User: $USER

Russ

[rsc, 2013-09-12 00:00:21]

not lgtm

for the record, this is how we got here:

g% hg log -v -r 16094
changeset:   16094:ca2b2e58fb69
user:        Russ Cox <rsc@golang.org>
date:        Wed Mar 06 16:48:20 2013 -0500
files:       src/pkg/net/http/export_test.go
src/pkg/net/http/header_test.go src/pkg/net/http/request.go
src/pkg/net/http/request_test.go src/pkg/net/http/requestwrite_test.go
description:
net/http: change user agent string

Some IDS somewhere thinks "Go http package" is a virus.
Make it something else for Go 1.1. Dumb but easy.

R=golang-dev, bradfitz, minux.ma
CC=golang-dev
https://codereview.appspot.com/7532043


g%

[dsymonds, 2013-09-12 00:03:39]

On 12 September 2013 09:59, Russ Cox <rsc@golang.org> wrote:

> That is not the precedent I was trying to set with this. I was only working
> around dumb antivirus software I don't believe we should be advertising the
> version of the client, any more than we should be sending
>
>     User: $USER

$USER is a complete non-sequitur. Almost every HTTP client in
existence reports some version in the User-Agent (and RFCs recommend
it).

[rsc, 2013-09-12 00:09:00]

I put the version there to work around an anti-virus thing.
I do not want to keep bumping the version on each release.
People running at tip are reporting 1.1 already anyway.

Russ

[adg, 2013-09-12 00:10:59]

Can we change this CL to add a comment explaining that the existing user
agent is a nonce and shouldn't be updated in the future?

[dsymonds, 2013-09-12 00:11:16]

What about I just make it
  var defaultUserAgent = "Go " + runtime.Version() + " package http"
and then we never need to touch it again?

[rsc, 2013-09-12 00:12:49]

how many times do i need to say not lgtm?

[minux1, 2013-09-12 00:13:48]

On Sep 11, 2013 8:10 PM, "Andrew Gerrand" <adg@golang.org> wrote:
> Can we change this CL to add a comment explaining that the existing user
agent is a nonce and shouldn't be updated in the future?
i suggest that if we go this route, then use something that's not like a
version number.

[dsymonds, 2013-09-12 00:14:39]

If it's like that, fine. I'll drop this. I just thought we could have
a discussion with reasons and stuff.

[dsymonds, 2013-09-12 00:14:51]

*** Abandoned ***

[adg, 2013-09-12 00:15:30]

On 12 September 2013 10:13, minux <minux.ma@gmail.com> wrote:

> i suggest that if we go this route, then use something that's not like a
> version number.


I'd be in favor of using just "Go 1 package http" but I also am happy with
the status quo.

[rsc, 2013-09-12 00:16:15]

https://codereview.appspot.com/13263052