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

empty body on GET responses that Upgrade #333

Closed
doriantaylor opened this issue Jun 15, 2019 · 4 comments
Closed

empty body on GET responses that Upgrade #333

doriantaylor opened this issue Jun 15, 2019 · 4 comments

Comments

@doriantaylor
Copy link

Currently using em-http 1.1.5 to write something to rip through and harvest metadata from a bunch of links and it silently failed on one of them (hi @aristus), so I broke it out into a copy of the getting started example to see it would replicate:

require 'eventmachine'
require 'em-http'

EventMachine.run {
  http = EventMachine::HttpRequest.new('http://carlos.bueno.org/about.html').get

  http.errback { p 'Uh oh'; EM.stop }
  http.callback {
    p http.response_header.status
    p http.response
    
    EventMachine.stop
  }
}

Sure enough it does; it returns:

200
""

Wireshark says the data goes over the connection as expected. Response headers are

HTTP/1.1 200 OK
Date: Sat, 15 Jun 2019 00:39:56 GMT
Server: Apache
Upgrade: h2
Connection: Upgrade, close
Last-Modified: Fri, 13 May 2016 00:27:58 GMT
ETag: "10d4-532ae58bc7f80"
Accept-Ranges: bytes
Content-Length: 4308
Vary: Accept-Encoding
Content-Type: text/html

The only thing slightly unusual is the Upgrade header and Connection: Upgrade, close, which seems to replicate an empty body when I enable mod_http2 on my own Apache. Not sure where to look for a diagnosis, but again this is 1.1.5.

@doriantaylor
Copy link
Author

doriantaylor commented Jun 15, 2019

okay it appears the bug is somewhere in between Http::Parser and the C code, unless there is some configuration that can be twiddled. It seems like on_body is never called if there's an Upgrade: header.

@doriantaylor
Copy link
Author

ah yes looks like this squarely belongs to http_parser.rb. neeeeeeever mind!

@igrigorik
Copy link
Owner

igrigorik commented Jun 15, 2019 via email

@doriantaylor
Copy link
Author

Heads up, I compiled Http::Parser against the latest Joyent parser and it fixed my issue but yours will probably need some massaging around keepalive connections and protocol upgrading, as I'm noticing EPROTO errors on some SSL connections but not others when I have keepalive turned on. Good to keep an eye out when they release a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants