Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

something doesn't work in Go 1.6 that worked in Go 1.5 #15280

Closed
Dave-White-Rakuten opened this issue Apr 13, 2016 · 7 comments
Closed

something doesn't work in Go 1.6 that worked in Go 1.5 #15280

Dave-White-Rakuten opened this issue Apr 13, 2016 · 7 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@Dave-White-Rakuten
Copy link

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?

1.6 v 1.5

  1. What operating system and processor architecture are you using (go env)?

linux/amd64

  1. What did you do?
    If possible, provide a recipe for reproducing the error.
    A complete runnable program is good.
    A link on play.golang.org is best.

Switched from go 1.5 to 1.6

  1. What did you expect to see?

Things to keep working ...

  1. What did you see instead?

This is an issue that seems to involved Go, Haproxy, and Docker. I don't know for sure that http/2 is the reason, but it's my only suspect. We have an http server built with Go running in a docker container on an AWS ubuntu machine. If the server is built with Go 1.5, everything is fine. With 1.6, we get a 503 error code from haproxy. If the httpchk is disabled, everything starts working again. If haproxy is bypassed (by using the docker-machine ip), the server is working fine regardless of version. I tried disabling http/2 using GODEBUG env variables but that made no difference. We do have some servers built with 1.6 behind haproxy working but they are not inside Docker containers.

So, this appears to be an environment-specific issue triggered by go 1.6 but I can't say for sure it has anything to do with http/2. I only suspect it because it was introduced in go 1.6.

Some details that might matter: Haproxy is listening on port 80 and forwarding to the go server on its own ip listening on ":80".

@bradfitz
Copy link
Contributor

If you disabled HTTP/2 and it's still broken, why do you suspect HTTP/2?

What do haproxy's logs say?

@bradfitz bradfitz changed the title x/net/http2: something doesn't work in Go 1.6 that worked in Go 1.5 Apr 14, 2016
@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 14, 2016
@bradfitz bradfitz added this to the Unplanned milestone Apr 14, 2016
@Dave-White-Rakuten
Copy link
Author

I suspect it for little more than I can find absolutely nothing else to suspect. Going through what else has changed in Go 1.6, it's the only item that seems relevant.

Here are some logs from haproxy:
Apr 14 14:44:41 localhost haproxy[14618]: 50.201.229.10:58730 [14/Apr/2016:14:44:41.297] IN-HTTP HTTP-OUT/ 0/-1/-1/-1/0 503 212 - - SCDN 0/0/0/0/0 0/0 {} "GET /status HTTP/1.1"

When I got those two logs, the web server wasn't actually running. After launching it, there are no more haproxy logs produced.

I stopped that container and launched one that was identical except in which version of Go was used to build. And here's the log message I get then:

Apr 14 14:51:17 localhost haproxy[14618]: 50.201.229.10:58838 [14/Apr/2016:14:51:15.201] IN-HTTP HTTP-OUT/SYNC_PID1 2592/0/0/8/2600 200 379 - - --VN 1/1/0/1/0 0/0 {} "GET /status HTTP/1.1"

@bradfitz
Copy link
Contributor

In Go 1.6, we reject HTTP/1.1 invalid requests without Host headers, per the spec.

See 6e11f45 and #11206 and #13624

Does haproxy send such bogus requests? Take a tcpdump on either side and see what you see haproxy sending on the wire?

@Dave-White-Rakuten
Copy link
Author

That's it! Looks like our httpchk line should have had a host all along but Go was just ignoring the requirement until 1.6. Go's bug fix just exposed out haproxy bug. We're fixed. Thank you!

@bradfitz
Copy link
Contributor

Is this a bug in haproxy or a bug in your haproxy config?

I don't know haproxy. Can you paste here the before & after configs?

@Dave-White-Rakuten
Copy link
Author

The bug is in our config:

The old one:
option httpchk HEAD /status HTTP/1.1\r\nHost: \ ^

The new one:
option httpchk HEAD /status HTTP/1.1\r\nHost: localhost

"localhost" doesn't even seem necessary. It looks like any string at all can be there and it works fine. Our systems team is going to look into that part tho, and make sure this doesn't introduce any unexpected side effects.

@bradfitz
Copy link
Contributor

Hah: HAProxy documents this,

https://cbonte.github.io/haproxy-dconv/configuration-1.5.html#4-option%20httpchk

But it's pretty trashy how it overloads the <version> field:

option httpchk <method> <uri> <version>

<method>  is the optional HTTP method used with the requests. When not set,
          the "OPTIONS" method is used, as it generally requires low server
          processing and is easy to filter out from the logs. Any method
          may be used, though it is not recommended to invent non-standard
          ones.

<uri>     is the URI referenced in the HTTP requests. It defaults to " / "
          which is accessible by default on almost any server, but may be
          changed to any other URI. Query strings are permitted.

<version> is the optional HTTP version string. It defaults to "HTTP/1.0"
          but some servers might behave incorrectly in HTTP 1.0, so turning
          it to HTTP/1.1 may sometimes help. Note that the Host field is
          mandatory in HTTP/1.1, and as a trick, it is possible to pass it
          after "\r\n" following the version string.

HAProxy should just add the Host header automatically for HTTP/1.1, really. Or if it wants to provide a mechanism for additional headers, it should be part of the httpchk syntax, not tacking it onto <version>

But whatever. Wrong bug tracker for that.

@golang golang locked and limited conversation to collaborators Apr 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

3 participants