x/net/http2: Transport interop issue against Apache mod_h2? #16519

songgao · 2016-07-27T23:10:47Z

Please answer these questions before submitting your issue. Thanks!

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

go version go1.6.3 linux/amd64

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

GOARCH="amd64"
GOBIN="/home/songgao/gopath/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/songgao/gopath"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT="1"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

What did you do?

// main.go
package main

import (
    "io"
    "log"
    "net/http"
    "os"
    "time"
)

func main() {
    c := &http.Client{
        Timeout: 4 * time.Second,
    }
    log.Println("about to get ...")
    resp, err := c.Get("https://wohlford.co")
    if err != nil {
        log.Fatal(err)
    }
    io.Copy(os.Stdout, resp.Body)
}

songgao at Song-ThinkPad in /tmp 
$ GODEBUG=http2debug=2 go run main.go
2016/07/27 16:09:12 about to get ...
2016/07/27 16:09:12 http2: Transport failed to get client conn for wohlford.co:443: http2: no cached connection was available
2016/07/27 16:09:13 http2: Transport creating client conn to 184.106.215.246:443
2016/07/27 16:09:13 http2: Framer 0xc8203762c0: wrote SETTINGS len=18, settings: ENABLE_PUSH=0, INITIAL_WINDOW_SIZE=4194304, MAX_HEADER_LIST_SIZE=10485760
2016/07/27 16:09:13 http2: Framer 0xc8203762c0: wrote WINDOW_UPDATE len=4 (conn) incr=1073741824
2016/07/27 16:09:16 Get https://wohlford.co: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)
exit status 1

songgao at Song-ThinkPad in /tmp 
$ GODEBUG=http2client=0 go run main.go
2016/07/27 16:09:23 about to get ...
<!DOCTYPE html>
<html lang=en>
<head>
  <meta charset=utf-8 />
  <title>Wohlford &amp; Co.</title>
  <style title="text/css">body{background-color:#f2f1e6;margin:25% 0 0 0}div{text-align:center}a{color:#4c3c2f;text-decoration:none}</style>
</head>
<body>
  <div>
    <img src=xwohlford-co.png.pagespeed.ic.QfSuzO-s_o.png alt="Wohlford &amp; Co. Logo" width=555 height=69 />
  </div>
<script>if(window.parent==window){(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){(i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)})(window,document,'script','//www.google-analytics.com/analytics.js','ga');ga('create','UA-1898639-5','auto',{'siteSpeedSampleRate':100});ga('send','pageview');}</script></body>
</html>

What did you expect to see?
Falls back to HTTP/1.1 or errors
What did you see instead?
Hangs until timeout

The text was updated successfully, but these errors were encountered:

songgao · 2016-07-29T02:44:46Z

Just tried above code with go1.6 and HTTP/2 works fine. So it seems to be something introduced in (go1.6, go1.6.3].

wohlford · 2016-07-29T03:54:39Z

Ubuntu 14.04.4 LTS (amd64)
Apache 2.4.23
OpenSSL 1.0.2h
Nghttp2 1.13.0

/etc/apache2/sites-enabled/wohlford.co.conf:

<VirtualHost 184.106.215.246:80 [2001:470:c50c::1]:80>
  SuexecUserGroup "#2566" "#2129"
  ServerName wohlford.co
  ServerAlias www.wohlford.co
  DocumentRoot /home/wohlford.co/web

  ErrorLog /var/log/virtualmin/wohlford.co_error_log
  CustomLog /var/log/virtualmin/wohlford.co_access_log combined

  # HTTP/2
  <IfModule mod_http2.c>
    Protocols h2 h2c http/1.1
  </IfModule>

  # Cache
  <IfModule mod_cache_disk.c>
    CacheEnable disk /
    <IfModule mod_headers.c>
      Header setifempty Cache-Control "max-age=1" env=GATEWAY_INTERFACE
    </IfModule>
  </IfModule>

  # Enable Page Optimizations
  <IfModule pagespeed_module>
    ModPagespeed On
    ModPagespeedRewriteLevel CoreFilters
    #ModPagespeedEnableFilters add_instrumentation
    ModPagespeedEnableFilters insert_ga
    ModPagespeedAnalyticsID UA-1898639-5
    ModPagespeedEnableFilters inline_images
    <ModPagespeedIf spdy>
      ModPagespeedDisableFilters inline_images
    </ModPagespeedIf>
  </IfModule>

  <IfModule mod_rewrite.c>
    # Enable Rewrite Engine
    RewriteEngine on
    RewriteOptions InheritDown

    # Disable TRACE and TRACK
    RewriteCond %{REQUEST_METHOD} ^(TRACE|TRACK)
    RewriteRule .* - [F,NS]

    # Redirect all queries to wohlford.co
    RewriteCond %{HTTPS} !=on
    RewriteCond %{HTTP_HOST} !=wohlford.co [NC]
    RewriteRule ^/(.*) http://wohlford.co/$1 [R=301,L]

    # Redirect all encrypted queries to https://wohlford.co
    RewriteCond %{HTTPS} ==on
    RewriteCond %{HTTP_HOST} !=wohlford.co [NC]
    RewriteRule ^/(.*) https://wohlford.co/$1 [R=301,L]

    # Prevent Hotlinking
    RewriteCond %{HTTP_REFERER} !://wohlford.co/ [NC]
    RewriteCond %{HTTP_REFERER} !^$
    RewriteRule .\.(?:gif|jpe?g|png|bmp|ico|mov|mp4|wmv|mpe?g|mp3|ogg|swf|flv|pdf|docx?|xslx?|pptx?|zip|rar|gz|tgz|bz2|tbz|sitx?)$ - [F,L]
  </IfModule>

  <IfModule mod_ssl.c>
    <If "-T %{SSL_CIPHER}">
      <IfModule mod_headers.c>
        # Enable HSTS
        Header always set Strict-Transport-Security "max-age=15552000"

        # Public Key Pinning (HPKP)
        Header always set Public-Key-Pins "pin-sha256=\"V+sq7PzGY76Xm5KCr1c4UbSfQXAfRZjSbKIKhZEUI1s=\"; pin-sha256=\"AWBnEHm13/KDq7NWvRdWy+ESYII86+7Z20WO/G32ROs=\"; pin-sha256=\"ImP7Nf6Xq6zC1loswY5OMcem5s2M2p1i+qxV12bspG8=\"; max-age=15552000"
      </IfModule>
    </If>
    <ElseIf "-f '/home/wohlford.co/ssl.ca'">
      <IfModule mod_rewrite.c>
        # Switch to HTTPS if UA asks
        RewriteCond %{HTTP:Upgrade-Insecure-Requests} 1
        RewriteRule ^ https://wohlford.co%{REQUEST_URI} [R,L]

        RewriteCond %{THE_REQUEST} ^[A-Z]{3,9}\ /(.*)\ HTTP/ [NC]
        RewriteRule ^/?(wp-admin/|wp-login\.php) https://wohlford.co%{REQUEST_URI}%{QUERY_STRING} [R=301,QSA,L]
      </IfModule>
    </ElseIf>
  </IfModule>

  ScriptAlias /cgi-bin/ /home/wohlford.co/cgi-bin/
  <Directory /home/wohlford.co/cgi-bin>
    Require all granted
    Options None
    AllowOverride None Options=ExecCGI,Includes,IncludesNOEXEC,Indexes,MultiViews,SymLinksIfOwnerMatch
  </Directory>

  <Directory /home/wohlford.co/web/stats>
    AuthName "wohlford.co statistics"
    AuthType Basic
    AuthUserFile /home/wohlford.co/.stats-htpasswd
    Require valid-user
    <Files .stats-htpasswd>
      Deny from all
    </Files>
    Options None
    AllowOverride AuthConfig Limit Options=ExecCGI,Includes,IncludesNOEXEC,Indexes,MultiViews,SymLinksIfOwnerMatch
  </Directory>

  <Directory /home/wohlford.co/web>
    Require all granted
    Options +MultiViews +SymLinksIfOwnerMatch +ExecCGI
    AllowOverride AuthConfig Limit Nonfatal=Override Options=ExecCGI,Includes,IncludesNOEXEC,Indexes,MultiViews,SymLinksIfOwnerMatch
    AllowOverrideList Header RequestHeader SetEnvIf SetEnvIfNoCase \
      RewriteEngine RewriteOptions RewriteBase RewriteCond RewriteRule \
      Redirect RedirectTemp RedirectPermanent RedirectMatch \
      DefaultIcon AddDescription AddIcon AddIconByEncoding AddIconByType

    # WordPress Rewriting
#    RewriteBase /
#    RewriteCond %{REQUEST_FILENAME} !-f
#    RewriteCond %{REQUEST_FILENAME} !-d
#    RewriteRule . /index.php [L]

    <IfModule mod_fcgid.c>
      AddHandler fcgid-script .php
      AddHandler fcgid-script .php5.6
      AddHandler fcgid-script .php7.0
      AddType application/x-httpd-php .php
      FCGIWrapper /home/wohlford.co/fcgi-bin/php7.0.fcgi .php
      FCGIWrapper /home/wohlford.co/fcgi-bin/php5.6.fcgi .php5.6
      FCGIWrapper /home/wohlford.co/fcgi-bin/php7.0.fcgi .php7.0
    </IfModule>
  </Directory>

  <IfModule mod_fcgid.c>
    RemoveHandler .php
    RemoveHandler .php5.6
    RemoveHandler .php7.0
    IPCCommTimeout 20
    FcgidMaxRequestLen 1073741824
  </IfModule>
</VirtualHost>
<VirtualHost 184.106.215.246:443 [2001:470:c50c::1]:443>
  SuexecUserGroup "#2566" "#2129"
  ServerName wohlford.co
  ServerAlias www.wohlford.co
  DocumentRoot /home/wohlford.co/web

  ErrorLog /var/log/virtualmin/wohlford.co_error_log
  CustomLog /var/log/virtualmin/wohlford.co_access_log combined

  SSLEngine on
  SSLCertificateFile      /home/wohlford.co/ssl.cert
  SSLCertificateKeyFile   /home/wohlford.co/ssl.key
  SSLCertificateChainFile /home/wohlford.co/ssl.ca
  SSLCACertificateFile    /etc/ssl/certs/StartCom_Ltd.-ca.pem

  # HTTP/2
  <IfModule mod_http2.c>
    Protocols h2 h2c http/1.1
  </IfModule>

  # Cache
  <IfModule mod_cache_disk.c>
    CacheEnable disk /
    <IfModule mod_headers.c>
      Header setifempty Cache-Control "max-age=1" env=GATEWAY_INTERFACE
    </IfModule>
  </IfModule>

  # Enable Page Optimizations
  <IfModule pagespeed_module>
    ModPagespeed On
    ModPagespeedRewriteLevel CoreFilters
    #ModPagespeedEnableFilters add_instrumentation
    ModPagespeedEnableFilters insert_ga
    ModPagespeedAnalyticsID UA-1898639-5
    ModPagespeedEnableFilters inline_images
    <ModPagespeedIf spdy>
      ModPagespeedDisableFilters inline_images
    </ModPagespeedIf>
  </IfModule>

  <IfModule mod_rewrite.c>
    # Enable Rewrite Engine
    RewriteEngine on
    RewriteOptions InheritDown

    # Disable TRACE and TRACK
    RewriteCond %{REQUEST_METHOD} ^(TRACE|TRACK)
    RewriteRule .* - [F,NS]

    # Redirect all queries to wohlford.co
    RewriteCond %{HTTPS} !=on
    RewriteCond %{HTTP_HOST} !=wohlford.co [NC]
    RewriteRule ^/(.*) http://wohlford.co/$1 [R=301,L]

    # Redirect all encrypted queries to https://wohlford.co
    RewriteCond %{HTTPS} ==on
    RewriteCond %{HTTP_HOST} !=wohlford.co [NC]
    RewriteRule ^/(.*) https://wohlford.co/$1 [R=301,L]

    # Prevent Hotlinking
    RewriteCond %{HTTP_REFERER} !://wohlford.co/ [NC]
    RewriteCond %{HTTP_REFERER} !^$
    RewriteRule .\.(?:gif|jpe?g|png|bmp|ico|mov|mp4|wmv|mpe?g|mp3|ogg|swf|flv|pdf|docx?|xslx?|pptx?|zip|rar|gz|tgz|bz2|tbz|sitx?)$ - [F,L]
  </IfModule>

  <IfModule mod_ssl.c>
    <If "-T %{SSL_CIPHER}">
      <IfModule mod_headers.c>
        # Enable HSTS
        Header always set Strict-Transport-Security "max-age=15552000"

        # Public Key Pinning (HPKP)
        Header always set Public-Key-Pins "pin-sha256=\"V+sq7PzGY76Xm5KCr1c4UbSfQXAfRZjSbKIKhZEUI1s=\"; pin-sha256=\"AWBnEHm13/KDq7NWvRdWy+ESYII86+7Z20WO/G32ROs=\"; pin-sha256=\"ImP7Nf6Xq6zC1loswY5OMcem5s2M2p1i+qxV12bspG8=\"; max-age=15552000"
      </IfModule>
    </If>
    <ElseIf "-f '/home/wohlford.co/ssl.ca'">
      <IfModule mod_rewrite.c>
        # Switch to HTTPS if UA asks
        RewriteCond %{HTTP:Upgrade-Insecure-Requests} 1
        RewriteRule ^ https://wohlford.co%{REQUEST_URI} [R,L]

        RewriteCond %{THE_REQUEST} ^[A-Z]{3,9}\ /(.*)\ HTTP/ [NC]
        RewriteRule ^/?(wp-admin/|wp-login\.php) https://wohlford.co%{REQUEST_URI}%{QUERY_STRING} [R=301,QSA,L]
      </IfModule>
    </ElseIf>
  </IfModule>

  ScriptAlias /cgi-bin/ /home/wohlford.co/cgi-bin/
  <Directory /home/wohlford.co/cgi-bin>
    Require all granted
    Options None
    AllowOverride None Options=ExecCGI,Includes,IncludesNOEXEC,Indexes,MultiViews,SymLinksIfOwnerMatch
  </Directory>

  <Directory /home/wohlford.co/web/stats>
    AuthName "wohlford.co statistics"
    AuthType Basic
    AuthUserFile /home/wohlford.co/.stats-htpasswd
    Require valid-user
    <Files .stats-htpasswd>
      Deny from all
    </Files>
    Options None
    AllowOverride AuthConfig Limit Options=ExecCGI,Includes,IncludesNOEXEC,Indexes,MultiViews,SymLinksIfOwnerMatch
  </Directory>

  <Directory /home/wohlford.co/web>
    Require all granted
    Options +MultiViews +SymLinksIfOwnerMatch +ExecCGI
    AllowOverride AuthConfig Limit Nonfatal=Override Options=ExecCGI,Includes,IncludesNOEXEC,Indexes,MultiViews,SymLinksIfOwnerMatch
    AllowOverrideList Header RequestHeader SetEnvIf SetEnvIfNoCase \
      RewriteEngine RewriteOptions RewriteBase RewriteCond RewriteRule \
      Redirect RedirectTemp RedirectPermanent RedirectMatch \
      DefaultIcon AddDescription AddIcon AddIconByEncoding AddIconByType

    # WordPress Rewriting
#    RewriteBase /
#    RewriteCond %{REQUEST_FILENAME} !-f
#    RewriteCond %{REQUEST_FILENAME} !-d
#    RewriteRule . /index.php [L]

    <IfModule mod_fcgid.c>
      AddHandler fcgid-script .php
      AddHandler fcgid-script .php5.6
      AddHandler fcgid-script .php7.0
      AddType application/x-httpd-php .php
      FCGIWrapper /home/wohlford.co/fcgi-bin/php7.0.fcgi .php
      FCGIWrapper /home/wohlford.co/fcgi-bin/php5.6.fcgi .php5.6
      FCGIWrapper /home/wohlford.co/fcgi-bin/php7.0.fcgi .php7.0
    </IfModule>
  </Directory>

  <IfModule mod_fcgid.c>
    RemoveHandler .php
    RemoveHandler .php5.6
    RemoveHandler .php7.0
    IPCCommTimeout 20
    FcgidMaxRequestLen 1073741824
  </IfModule>
</VirtualHost>

Server Module List:

core.c
event.c
http_core.c
mod_access_compat.c
mod_actions.c
mod_alias.c
mod_auth_basic.c
mod_authn_core.c
mod_authn_file.c
mod_authz_core.c
mod_authz_groupfile.c
mod_authz_host.c
mod_authz_user.c
mod_autoindex.c
mod_cache.c
mod_cache_disk.c
mod_cache_socache.c
mod_cgid.c
mod_deflate.c
mod_dir.c
mod_encoding.c
mod_env.c
mod_expires.c
mod_fcgid.c
mod_filter.c
mod_headers.c
mod_http2.c
mod_info.c
mod_instaweb.cc
mod_log_config.c
mod_logio.c
mod_mime.c
mod_negotiation.c
mod_proxy.c
mod_proxy_fcgi.c
mod_proxy_html.c
mod_proxy_http.c
mod_reqtimeout.c
mod_rewrite.c
mod_setenvif.c
mod_so.c
mod_socache_shmcb.c
mod_ssl.c
mod_status.c
mod_suexec.c
mod_unixd.c
mod_version.c
mod_watchdog.c
mod_xml2enc.c

bradfitz · 2016-07-29T06:55:53Z

@icing, we might have an interop problem.

@songgao, there's no obvious place to fallback here. There's no great way to distinguish between an interop problem (if that's what it is... some bug on either side, or both), or an actual slow response hitting your timeout. Falling back h2 to h1 whenever an h2 response was legitimately slow doesn't work. Especially for general http retry reasons. (non-idempotent methods, etc)

I'll work on a preparing a Docker container with mod_h2 so I can reproduce this, unless somebody already has one.

@songgao, do you have a minimal repro?

icing · 2016-07-29T08:46:25Z

@bradfitz I see hanging request with my go 1.6.2. In order to check against my local test server, how do i disable the cert verification? :-D

bradfitz · 2016-07-29T08:50:48Z

@icing, you can write code like:

import (
   "crypto/tls"
   "log"
   "net/http"
   "os"

   "golang.org/x/net/http2"
)

func main() {
    c := &http.Client{
        Transport: &http2.Transport{
              TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
        },
    }
    res, err := c.Get("https://localhost:1234/foo.txt")
    if err != nil { log.Fatal(err) }
    res.Write(os.Stdout)
}

And if you run it with GODEBUG=http2debug=2 set in your environment, it will dump out lots of information to stderr.

Thanks for looking!

I'll investigate more when I'm home.

icing · 2016-07-29T09:12:42Z

@bradfitz thanks!

I think you are running into the issue fixed in v1.5.12:

when the client waits on a SETTINGS ACK without anything else going on, it might get stuck in h2's output buffers.

As nghttp and curl always open stream 1 without waiting for the ack, this was not noticed. Have to add go tests some day...

bradfitz · 2016-07-29T09:18:09Z

Perhaps Go could send a dummy OPTIONS request if no settings response is
heard after an RTT or so? That should force a flush on the mod_h2 side?

On Jul 29, 2016 11:12 AM, "Stefan Eissing" notifications@github.com wrote:

@bradfitz https://github.com/bradfitz thanks!

I think you are running into the issue fixed in v1.5.12
https://github.com/icing/mod_h2/releases/tag/v1.5.12:

when the client waits on a SETTINGS ACK without anything else going on, it
might get stuck in h2's output buffers.

As nghttp and curl always open stream 1 without waiting for the ack, this
was not noticed. Have to add go tests some day...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16519 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAKPb3toD6l9yBNM8XkTrxk1s-MMv8fks5qacQLgaJpZM4JWt--
.

icing · 2016-07-29T09:21:57Z

Or send the first request right away? Save your client 1 RTT..

songgao · 2016-07-29T14:58:40Z

@bradfitz Thanks for the explanation. I don't have a minimal repro yet; sorry!

songgao · 2016-07-29T17:27:34Z

So, out of curiosity, I was wondering why go1.6 works with the same server. Does go1.6 always open stream 1 without waiting for the ack like nghttp/curl? If so, was the reason for changing that "compliance with standard"?

bradfitz · 2016-07-29T18:33:18Z

@songgao, Go 1.6 had a bug where HTTP/2 was enabled by default on all HTTP clients except the default one, embarrassingly. It was a bug introduced almost immediately before Go 1.6 was released, in the process of fixing a different bug, embarrassingly. It was fixed in Go 1.6.2 (Go 1.6.1 was a security-only release).

So you weren't using Go's http2 stack with Go 1.6 probably.

@icing, yeah, I suppose I could do that too. But would a PING frame force a flush of your buffers? That might be lower-risk at this stage of our release, which is just around the corner.

icing · 2016-07-30T07:21:03Z

If I understand my code, only a request will bring it out of that embarrassing state of not flushing its buffers then. It could be any request, really. Maybe insert a dummy one? Sorry about that.

icing · 2016-07-30T08:56:51Z

Started adding go tests into my test suite. Seeing other interop issues, e.g. where Apache sends response HEADER + DATA (eof set) and the go client hangs. No flow involved. And go on http1 works fine.

Does it makes sense to test with 1.6.2? Will have a closer look next week...

bradfitz · 2016-07-30T17:06:11Z

If I understand my code, only a request will bring it out of that embarrassing state of not flushing its buffers then. It could be any request, really. Maybe insert a dummy one? Sorry about that.

I thought about sending a dummy OPTIONS request, but then I found it was easier to just not wait for the peer's settings & settings ACK instead. I hacked that up on the plane ride home. It might be too late for Go 1.7.

Started adding go tests into my test suite. Seeing other interop issues, e.g. where Apache sends response HEADER + DATA (eof set) and the go client hangs. No flow involved. And go on http1 works fine.

File a new Go bug and post details? I need to setup mod_h2 interop tests on my side too, but do share any Go snippet you find that hangs.

Does it makes sense to test with 1.6.2? Will have a closer look next week...

Either use Go 1.6.2, or use Go 1.6 and just make sure you create your own *http.Client instead of relying on the http.DefaultClient ...

     c:= &http.Client{Transport: &http.Transport{}}
     c.Get(....)

wohlford · 2016-07-30T17:43:27Z

I've been tracking this issue and I'm afraid some of the technical details are over me head. Pardon if I ask questions that don't really make all that much sense. ;-)

The original symptom came up with my use of keybase.io. It would validate my identity on my website over https, but not http. I mentioned it to @maxtaco. I believe he followed up with @songgao, and now there's this issue on GitHub. And frankly all that is so damn cool! I love open source! :-)

If I follow everything above, it breaks when using http2. I've disabled my http2 module in apache and restarted the daemon. So why can't I still validate?

Works: https://wohlford.co/.well-known/keybase.txt

Doesn't work: http://wohlford.co/.well-known/keybase.txt

I'm thinking there is something else going on here besides h2/h2c issues. Don't want us to miss that if true.

songgao · 2016-07-30T18:12:57Z

Thanks for looking into it @wohlford (Also thanks @bradfitz for the fantastic explanation!)

Jason, I think this thread is more for Go's net/http package. Let's move the Keybase related discussion to keybase/client#3656 :D

gopherbot · 2016-07-30T21:00:25Z

CL https://golang.org/cl/25362 mentions this issue.

songgao · 2016-07-30T21:30:25Z

@bradfitz Thanks for the fix! Any chance the patch will make it into go1.7?

bradfitz · 2016-07-30T21:41:35Z

@songgao, it's pretty late in the cycle, but it's pretty crappy for users as-is so we probably should. I verified that this makes Go work with the latest Debian squeeze apache2, which doesn't yet have the upstream mod_h2 fix yet.

@adg, @broady, @quentinmit, @ianlancetaylor... thoughts?

quentinmit · 2016-07-31T01:33:40Z

My vote is to put it in 1.7. It's pretty annoying if we enable HTTP2 by default and we can't interoperate with Apache.

adg · 2016-08-01T00:59:30Z

Where do we draw the line for these changes?

bradfitz · 2016-08-01T01:03:26Z

There aren't many http2 implementations. I think it's pretty bad for users if they don't interop, even if it's temporary.

Plus I think the proposed change is how we want things anyway.

adg · 2016-08-01T01:05:34Z

OK

On 1 August 2016 at 11:04, Brad Fitzpatrick notifications@github.com
wrote:

There aren't many http2 implementations. I think it's pretty bad for users
if they don't interop, even if it's temporary.

Plus I think the proposed change is how we want things anyway.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16519 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AIDilY_qLW_iKJhrySnVHIKOYone3RO2ks5qbUYJgaJpZM4JWt--
.

gopherbot · 2016-08-02T00:00:18Z

CL https://golang.org/cl/25388 mentions this issue.

Updates bundled http2 to x/net/http2 rev 28d1bd4f for: http2: make Transport work around mod_h2 bug https://golang.org/cl/25362 http2: don't ignore DATA padding in flow control https://golang.org/cl/25382 Updates #16519 Updates #16556 Updates #16481 Change-Id: I51f5696e977c91bdb2d80d2d56b8a78e3222da3f Reviewed-on: https://go-review.googlesource.com/25388 Reviewed-by: Chris Broadfoot <cbro@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>

songgao · 2016-08-02T00:52:58Z

Thanks for getting this in before 1.7 release!

icing · 2016-08-02T07:59:41Z

Thanks for the quick fix!

gopherbot · 2016-08-05T07:00:31Z

CL https://golang.org/cl/25508 mentions this issue.

The http2 spec says: > When the value of SETTINGS_INITIAL_WINDOW_SIZE changes, a receiver > MUST adjust the size of all stream flow-control windows that it > maintains by the difference between the new value and the old value. We didn't do this before, and it never mattered until https://golang.org/cl/25362 for golang/go#16519 because we always knew the peer's initial window size. Once we started writing request bodies before hearing the peer's setting (and thus assuming 64KB), it became very important that this TODO was done. Should've done it earlier. More details in the bug. Updates golang/go#16612 (fixes after bundle into std) Change-Id: I0ac0280bdd5f6e933ad82f8c9df3c4528295bac2 Reviewed-on: https://go-review.googlesource.com/25508 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org>

The http2 server implementation in Apache (mod_http2, aka mod_h2) had a bug where it didn't reply to the initial SETTINGS frame until it had received a request. Apache 2.4.17 was the first Apache version to include mod_http2 and has the bug. The bug was fixed 20 days ago (apache/httpd@de4e303) for Apache 2.5.0 and included in the out-of-tree mod_h2 1.5.12 (https://github.com/icing/mod_h2/releases/tag/v1.5.12) but most Apache sites are running older versions making them incompatible with Go's client. (At least Debian Stretch and Ubuntu Xenial, currently.) Chrome, curl, Firefox et al don't wait for the initial SETTINGS response & ACK before sending their first request, which is why mod_http2 didn't notice their bug for some time. This change makes Go's http2.Transport act like other common HTTP/2 clients and not wait for the peer's SETTINGS frame & ACK before sending the first request. As a bonus, this reduces the latency for the first request on connections by one RTT. The reason we waited for the initial settings is purely historical. Andrew and I just wrote it that way when initially developing the client on video (https://www.youtube.com/watch?v=yG-UaBJXZ80) because we were working top-down through the protocol and didn't get back to optimizing the RTT away. It also seemed more compliant to wait for the peer's SETTINGS to know the frame size and such, but as as spec says, it's permitted for clients to not wait: http://httpwg.org/specs/rfc7540.html#rfc.section.3.5 > To avoid unnecessary latency, clients are permitted to send > additional frames to the server immediately after sending the client > connection preface, without waiting to receive the server connection > preface. It is important to note, however, that the server > connection preface SETTINGS frame might include parameters that > necessarily alter how a client is expected to communicate with the > server. Upon receiving the SETTINGS frame, the client is expected to > honor any parameters established. In some configurations, it is > possible for the server to transmit SETTINGS before the client sends > additional frames, providing an opportunity to avoid this issue. So, don't wait. Fixes golang/go#16519 Change-Id: I916827e49829f7f107a51838512816916fb553fc Reviewed-on: https://go-review.googlesource.com/25362 Reviewed-by: Andrew Gerrand <adg@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>

songgao changed the title ~~http.Get hangs if HTTP/2 is enabled~~ http.Get hangs for some server if HTTP/2 is enabled Jul 27, 2016

songgao mentioned this issue Jul 29, 2016

disable HTTP/2 as a temporary fix for CORE-3441 keybase/client#3656

Merged

songgao changed the title ~~http.Get hangs for some server if HTTP/2 is enabled~~ x/net/http2: http.Get hangs for some server Jul 29, 2016

bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 29, 2016

bradfitz changed the title ~~x/net/http2: http.Get hangs for some server~~ x/net/http2: Transport interop issue against Apache mod_h2? Jul 29, 2016

bradfitz removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 30, 2016

bradfitz added this to the Go1.7Maybe milestone Jul 30, 2016

gopherbot closed this as completed in golang/net@28d1bd4 Aug 1, 2016

This was referenced Aug 2, 2016

x/net/http2: Transport hangs on malformed response headers #16572

Closed

x/net/http2: hung request to vision.googleapis.com (bad flow control?) #16612

Closed

golang locked and limited conversation to collaborators Aug 5, 2017

gopherbot added the FrozenDueToAge label Aug 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/net/http2: Transport interop issue against Apache mod_h2? #16519

x/net/http2: Transport interop issue against Apache mod_h2? #16519

songgao commented Jul 27, 2016

songgao commented Jul 29, 2016

wohlford commented Jul 29, 2016

bradfitz commented Jul 29, 2016

icing commented Jul 29, 2016

bradfitz commented Jul 29, 2016 •

edited

Loading

icing commented Jul 29, 2016

bradfitz commented Jul 29, 2016

icing commented Jul 29, 2016 •

edited

Loading

songgao commented Jul 29, 2016

songgao commented Jul 29, 2016

bradfitz commented Jul 29, 2016

icing commented Jul 30, 2016

icing commented Jul 30, 2016

bradfitz commented Jul 30, 2016

wohlford commented Jul 30, 2016

songgao commented Jul 30, 2016

gopherbot commented Jul 30, 2016

songgao commented Jul 30, 2016

bradfitz commented Jul 30, 2016

quentinmit commented Jul 31, 2016

adg commented Aug 1, 2016

bradfitz commented Aug 1, 2016

adg commented Aug 1, 2016

gopherbot commented Aug 2, 2016

songgao commented Aug 2, 2016

icing commented Aug 2, 2016

gopherbot commented Aug 5, 2016

x/net/http2: Transport interop issue against Apache mod_h2? #16519

x/net/http2: Transport interop issue against Apache mod_h2? #16519

Comments

songgao commented Jul 27, 2016

songgao commented Jul 29, 2016

wohlford commented Jul 29, 2016

bradfitz commented Jul 29, 2016

icing commented Jul 29, 2016

bradfitz commented Jul 29, 2016 • edited Loading

icing commented Jul 29, 2016

bradfitz commented Jul 29, 2016

icing commented Jul 29, 2016 • edited Loading

songgao commented Jul 29, 2016

songgao commented Jul 29, 2016

bradfitz commented Jul 29, 2016

icing commented Jul 30, 2016

icing commented Jul 30, 2016

bradfitz commented Jul 30, 2016

wohlford commented Jul 30, 2016

songgao commented Jul 30, 2016

gopherbot commented Jul 30, 2016

songgao commented Jul 30, 2016

bradfitz commented Jul 30, 2016

quentinmit commented Jul 31, 2016

adg commented Aug 1, 2016

bradfitz commented Aug 1, 2016

adg commented Aug 1, 2016

gopherbot commented Aug 2, 2016

songgao commented Aug 2, 2016

icing commented Aug 2, 2016

gopherbot commented Aug 5, 2016

bradfitz commented Jul 29, 2016 •

edited

Loading

icing commented Jul 29, 2016 •

edited

Loading