code review 6570045: net/http: use tls host instead of proxy, when checking ...

net/http: use tls host instead of proxy, when checking against a certificate

Fixes issue 4152.

[chressie1, 2012-09-25 14:55:18]

Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),

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

[bradfitz, 2012-09-25 16:22:59]

LGTM

Wish this had a test, but there's not a simple way to modify an existing
test to cover this, so I'm happy that "go get" now works via a proxy.

Thanks!


On Tue, Sep 25, 2012 at 7:55 AM, <chressie@gmail.com> wrote:

> Reviewers: bradfitz,
>
> Message:
> Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> net/http: use tls host instead of proxy, when checking against a
> certificate
>
> Fixes issue 4152.
>
> Please review this at
http://codereview.appspot.com/**6570045/<http://codereview.appspot.com/6570045/>
>
> Affected files:
>   M src/pkg/net/http/transport.go
>
>
> Index: src/pkg/net/http/transport.go
> ==============================**==============================**=======
> --- a/src/pkg/net/http/transport.**go
> +++ b/src/pkg/net/http/transport.**go
> @@ -381,7 +381,7 @@
>                 // Initiate TLS and check remote host name against
> certificate.
>                 cfg := t.TLSClientConfig
>                 if cfg == nil || cfg.ServerName == "" {
> -                       host, _, _ := net.SplitHostPort(cm.addr())
> +                       host := cm.tlsHost()
>                         if cfg == nil {
>                                 cfg = &tls.Config{ServerName: host}
>                         } else {
>
>
>

[bradfitz, 2012-09-25 16:23:17]

*** Submitted as http://code.google.com/p/go/source/detail?r=c70d7dfc3d07 ***

net/http: use tls host instead of proxy, when checking against a certificate

Fixes issue 4152.

R=bradfitz
CC=golang-dev
http://codereview.appspot.com/6570045

Committer: Brad Fitzpatrick <bradfitz@golang.org>

[chressie1, 2012-09-26 12:19:40]

On 2012/09/25 16:22:59, bradfitz wrote:
> Wish this had a test, but there's not a simple way to modify an existing
> test to cover this, so I'm happy that "go get" now works via a proxy.

yes, i didn't see a simple way as well. it would be nice to have a proxy
implementation and i think it's not feasible to use an open proxy in the web for
testing that.

cheers..

[bradfitz, 2012-09-26 15:25:26]

On Wed, Sep 26, 2012 at 5:19 AM, <chressie@gmail.com> wrote:

> On 2012/09/25 16:22:59, bradfitz wrote:
>
>> Wish this had a test, but there's not a simple way to modify an
>>
> existing
>
>> test to cover this, so I'm happy that "go get" now works via a proxy.
>>
>
> yes, i didn't see a simple way as well. it would be nice to have a proxy
> implementation and i think it's not feasible to use an open proxy in the
> web for testing that.
>

shouldn't be hard with httputil's reverse proxy code.  want to work on one
to be used in the tests?  then we can pass it flags to have a "fake DNS"
mode where certain hostnames are hard-coded to be localhost for tests.

[chressie1, 2012-09-26 16:44:55]

On Wed, Sep 26, 2012 at 5:25 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> shouldn't be hard with httputil's reverse proxy code.  want to work on one
> to be used in the tests?  then we can pass it flags to have a "fake DNS"
> mode where certain hostnames are hard-coded to be localhost for tests.

cool, if it's not too urgent to have a test here, i'd like to take a
look when i have some spare time.