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

HTTP::ConnectionError on HTTPS queries through HTTP proxies #685

Closed
ClearlyClaire opened this issue Sep 9, 2021 · 9 comments · Fixed by #686
Closed

HTTP::ConnectionError on HTTPS queries through HTTP proxies #685

ClearlyClaire opened this issue Sep 9, 2021 · 9 comments · Fixed by #686

Comments

@ClearlyClaire
Copy link

Hi,
We are using the http gem in Mastodon, and I've been reported failures when using a HTTP proxy.

Upon investigation, it seems it occurs directly in the http gem and only fails with HTTPS URLs.

Indeed, the following works:

HTTP.via('127.0.0.1', 8118).get('http://httpbin.org/get')

But the following fails:

HTTP.via('127.0.0.1', 8118).get('https://httpbin.org/get')

It fails with the following error:

/home/mastodon/live/vendor/bundle/ruby/3.0.0/gems/http-5.0.1/lib/http/connection.rb:108:in `read_headers!': couldn't read response headers (HTTP::ConnectionError)
	from /home/mastodon/live/vendor/bundle/ruby/3.0.0/gems/http-5.0.1/lib/http/client.rb:76:in `perform'
	from /home/mastodon/live/vendor/bundle/ruby/3.0.0/gems/httplog-1.5.0/lib/httplog/adapters/http.rb:12:in `block (2 levels) in <class:Client>'
	from /usr/lib/ruby/3.0.0/benchmark.rb:308:in `realtime'
	from /home/mastodon/live/vendor/bundle/ruby/3.0.0/gems/httplog-1.5.0/lib/httplog/adapters/http.rb:11:in `block in <class:Client>'
	from /home/mastodon/live/vendor/bundle/ruby/3.0.0/gems/http-5.0.1/lib/http/client.rb:31:in `request'
	from /home/mastodon/live/vendor/bundle/ruby/3.0.0/gems/http-5.0.1/lib/http/chainable.rb:20:in `get'
	from (irb):1:in `<main>'

This also fails with http 5.0.0 but not with 4.4. Version 5.0.0pre3 seems to also work.

@ClearlyClaire
Copy link
Author

git bisecting it points to 8f8682c introducing the issue

@tarcieri
Copy link
Member

tarcieri commented Sep 9, 2021

cc @bryanp

@bryanp
Copy link
Contributor

bryanp commented Sep 9, 2021

I'm able to reproduce—investigating.

@bryanp
Copy link
Contributor

bryanp commented Sep 9, 2021

Okay, I traced the issue all the way back to the llhttp library that the llhttp gem provides bindings to: nodejs/llhttp#128

This is causing our handler to be in an incorrect state. I'll report back once I hear back from the llhttp folks.

@bryanp
Copy link
Contributor

bryanp commented Sep 9, 2021

It looks to be an issue with the implementation in #651. We need to reset the parser rather than simply call finish.

I'll be releasing an update to llhttp-ffi today or tomorrow and will PR a corresponding change here.

@bryanp
Copy link
Contributor

bryanp commented Sep 9, 2021

Well, it all happened faster than I expected: #686. I'll let @tarcieri make the call on when to cut a release.

@tarcieri
Copy link
Member

tarcieri commented Sep 9, 2021

@ClearlyClaire I've merged a prospective fix in #686. Can you confirm it fixes your problem?

@ClearlyClaire
Copy link
Author

Yes, I can confirm this solves my issue!

@tarcieri
Copy link
Member

Released in 5.0.2

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

Successfully merging a pull request may close this issue.

3 participants