Skip to content

Replace http-parser with llhttp#651

Merged
ixti merged 4 commits intohttprb:masterfrom
bryanp:llhttp
Mar 4, 2021
Merged

Replace http-parser with llhttp#651
ixti merged 4 commits intohttprb:masterfrom
bryanp:llhttp

Conversation

@bryanp
Copy link
Contributor

@bryanp bryanp commented Mar 4, 2021

Builds on #639 to finish updating the response parser to use llhttp (using the ffi implementation for broader compatibility).

I updated one spec as skipped as llhttp does not return an error like http-parser does. Best I can tell llhttp exhibits the correct behavior, which was also @midnight-wonderer's take here: #639 (comment). Let me know if you disagree and I'll start a discussion with the authors of the llhttp c library.

Resolves #604, #622.

@bryanp bryanp mentioned this pull request Mar 4, 2021
Closed
@bryanp
Copy link
Contributor Author

bryanp commented Mar 4, 2021

@ixti or @tarcieri: how do you feel about dropping Ruby 2.4 support with this change? Ruby 2.4 is no longer maintained (2.5 will EOL soon as well). IMO this seems like a good opportunity to drop support for old rubies.

Right now https://github.com/metabahn/llhttp only supports Ruby 2.5 and up (explaining the failing checks above). I don't think there's any reason it couldn't support Ruby 2.4, but I'd prefer to only support maintained rubies. Let me know your thoughts.

@tarcieri
Copy link
Member

tarcieri commented Mar 4, 2021 •
edited
Loading

I'm fine with bumping the minimum Ruby version to 2.5, or even 2.6 if 2.5 is about to be EOL.

@ixti
Copy link
Member

ixti commented Mar 4, 2021

Yeah with 5.x release I was hoping to bump the min ruby version.

@ixti
Copy link
Member

ixti commented Mar 4, 2021

I'm gonna merge #652 and then will rebase&merge this PR

@ixti
Copy link
Member

ixti commented Mar 4, 2021

Need to support Ruby 2.5 due to jRuby :((

@bryanp
Copy link
Contributor Author

bryanp commented Mar 4, 2021

Speaking of jRuby, I'm trying to debug the failure above but can't reproduce locally. Have you seen this before?

exec: jrake: not found

Not sure why it's trying to use jrake—I don't see a reference to it in ffi-compiler.

@ixti
Copy link
Member

ixti commented Mar 4, 2021

It's not related to ffi-compiler. My branch with no other changes but ruby drop - fails on jruby as well o_O

@tarcieri
Copy link
Member

tarcieri commented Mar 4, 2021

@enebo @headius any idea what might be causing these jrake: not found errors which ostensibly appear to be coming out of ffi-compiler (even though that doesn't appear to directly reference jrake anywhere in its source code?)

@ixti
Copy link
Member

ixti commented Mar 4, 2021

@tarcieri I propose to temporarily exclude jRuby from the CI and deal with it later

@ixti
Copy link
Member

ixti commented Mar 4, 2021

@bryanp please rebase on master and I'll merge you PR. Will deal with jRuby later

@tarcieri
Copy link
Member

tarcieri commented Mar 4, 2021 •
edited
Loading 

lib/http/response/parser.rb:13:48: C: [Correctable] Style/HashSyntax: Use hash rockets syntax.
        @parser = LLHttp::Parser.new(@handler, type: :response)

@ixti it might be time to consider migrating to using the Ruby 1.9 syntax

@bryanp
Copy link
Contributor Author

bryanp commented Mar 4, 2021

@ixti done

@headius
Copy link

headius commented Mar 4, 2021

what might be causing these jrake: not found errors

That is a new one to me... we have not shipped a "jrake" in years and years. There used to be a push (by others, not by us) to prefix every command installed in JRuby with a "j" like "jgem", "jrake", "jrails" and so on, so perhaps ffi-compiler got caught up in that?

@tarcieri
Copy link
Member

tarcieri commented Mar 4, 2021

@headius the confusing thing is I don't see jrake referenced anywhere in the ffi-compiler source code, nor do any of its dependencies (of which it only has a couple) stick out at me in particular as potential culprits

@ixti ixti merged commit 8f8682c into httprb:master Mar 4, 2021
@ixti
Copy link
Member

ixti commented Mar 4, 2021

Thank you!

@bryanp bryanp deleted the llhttp branch March 4, 2021 19:42
@avokhmin avokhmin mentioned this pull request Mar 5, 2021
Closed
@brodock brodock mentioned this pull request Apr 23, 2021
Closed
@bryanp
Copy link
Contributor Author

bryanp commented May 6, 2021

@ixti Any idea when the next release might be pushed out?

@tarcieri
Copy link
Member

tarcieri commented May 10, 2021

See #659 and #660

@tarcieri tarcieri mentioned this pull request May 13, 2021
Merged
@ixti ixti mentioned this pull request May 19, 2021
Open
@bryanp bryanp mentioned this pull request Sep 9, 2021
Closed
@JDutil JDutil mentioned this pull request Aug 21, 2023
Closed
@tarcieri tarcieri mentioned this pull request Mar 21, 2025
Closed
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 11, 2025
@ryoon
www/ruby-http: Update to 5.3.1
1478a6c 
Changelog:
## [5.3.1] - 2025-06-09

### Changed

- Revert switch to the native llhttp on MRI, as it's not compatible with
  standalone bundles
  ([#802](httprb/http#802))


## [5.3.0] - 2025-06-09

### Added

- (backported) Add .retriable feature to Http
- (backported) Add more specific ConnectionError classes
- (backported) New feature: RaiseError

### Changed

- (backported) Drop depenency on base64
- (backported) Cache header normalization to reduce object allocation
- (backported) Use native llhttp on MRI


## [5.2.0] - 2024-02-05

### Added

- Add `Connection#finished_request?`
  ([#743](httprb/http#743))
- Add `Instrumentation#on_error`
  ([#746](httprb/http#746))
- Add `base64` dependency (suppresses warnings on Ruby 3.0)
  ([#759](httprb/http#759))
- Add `PURGE` HTTP verb
  ([#757](httprb/http#757))
- Add Ruby-3.3 support

### Changed

- **BREAKING** Process features in reverse order
  ([#766](httprb/http#766))
- **BREAKING** Downcase Content-Type charset name
  ([#753](httprb/http#753))
- **BREAKING** Make URI normalization more conservative
  ([#758](httprb/http#758))

### Fixed

- Close sockets on initialize failure
  ([#762](httprb/http#762))
- Prevent CRLF injection due to broken URL normalizer
  ([#765](httprb/http#765))

[unreleased]: httprb/http@v5.3.0...5-x-stable
[5.3.0]: httprb/http@v5.2.0...v5.3.0
[5.2.0]: httprb/http@v5.1.1...v5.2.0

## 5.1.1 (2022-12-17)

* [#731](httprb/http#731)
  Strip brackets from IPv6 addresses in `HTTP::URI`.
  ([@jeraki])

* [#722](httprb/http#722)
  Add `on_redirect` callback.
  ([@benubois])

## 5.1.0 (2022-06-17)

* Drop ruby-2.5 support.

* [#715](httprb/http#715)
  Set default encoding to UTF-8 for `application/json`.
  ([@drwl])

* [#712](httprb/http#712)
  Recognize cookies set by redirect.
  ([@tkellogg])

* [#707](httprb/http#707)
  Distinguish connection timeouts.
  ([@YuLeven])

## 5.0.4 (2021-10-07)

* [#698](httprb/http#698)
  Fix `HTTP::Timeout::Global#connect_ssl`.
  ([@tarcieri])

## 5.0.3 (2021-10-06)

* [#695](httprb/http#695)
  Revert DNS resolving feature.
  ([@PhilCoggins])

* [#694](httprb/http#694)
  Fix cookies extraction.
  ([@flosacca])

## 5.0.2 (2021-09-10)

* [#686](httprb/http#686)
  Correctly reset the parser.
  ([@bryanp])

* [#684](httprb/http#684)
  Don't set Content-Length for GET, HEAD, DELETE, or CONNECT requests without a BODY.
  ([@jyn514])

* [#679](httprb/http#679)
  Use features on redirected requests.
  ([@nomis])

* [#678](httprb/http#678)
  Restore `HTTP::Response` `:uri` option for backwards compatibility.
  ([@schwern])

* [#676](httprb/http#676)
  Update addressable because of CVE-2021-32740.
  ([@matheussilvasantos])

* [#653](httprb/http#653)
  Avoid force encodings on frozen strings.
  ([@bvicenzo])

* [#638](httprb/http#638)
  DNS failover handling.
  ([@midnight-wonderer])


## 5.0.1 (2021-06-26)

* [#670](httprb/http#670)
  Revert `Response#parse` behavior introduced in [#540].
  ([@DannyBen])

* [#669](httprb/http#669)
  Prevent bodies from being resubmitted when following unsafe redirects.
  ([@odinhb])

* [#664](httprb/http#664)
  Bump llhttp-ffi to 0.3.0.
  ([@bryanp])


## 5.0.0 (2021-05-12)

* [#656](httprb/http#656)
  Handle connection timeouts in `Features`
  ([@semenyukdmitry])

* [#651](httprb/http#651)
  Replace `http-parser` with `llhttp`
  ([@bryanp])

* [#647](httprb/http#647)
  Add support for `MKCALENDAR` HTTP verb
  ([@meanphil])

* [#632](httprb/http#632)
  Respect the SSL context's `verify_hostname` value
  ([@colemannugent])

* [#625](httprb/http#625)
  Fix inflator with empty responses
  ([@LukaszMaslej])

* [#599](httprb/http#599)
  Allow passing `HTTP::FormData::{Multipart,UrlEncoded}` object directly.
  ([@ixti])

* [#593](httprb/http#593)
  [#592](httprb/http#592)
  Support informational (1XX) responses.
  ([@ixti])

* [#590](httprb/http#590)
  [#589](httprb/http#589)
  Fix response headers paring.
  ([@Bonias])

* [#587](httprb/http#587)
  [#585](httprb/http#585)
  Fix redirections when server responds with multiple Location headers.
  ([@ixti])

* [#581](httprb/http#581)
  [#582](httprb/http#582)
  Add Ruby 2.7.x support.
  ([@janko])

* [#577](httprb/http#577)
  Fix `Chainable#timeout` with frozen Hash.
  ([@antonvolkoff])

* [#576](httprb/http#576)
  [#524](httprb/http#524)
  **BREAKING CHANGE**
  Preserve header names casing.
  ([@joshuaflanagan])

* [#540](httprb/http#540)
  [#538](httprb/http#538)
  **BREAKING CHANGE**
  Require explicit MIME type for Response#parse
  ([@ixti])

* [#532](httprb/http#532)
  Fix pipes support in request bodies.
  ([@ixti])

* [#530](httprb/http#530)
  Improve header fields name/value validation.
  ([@Bonias])

* [#506](httprb/http#506)
  [#521](httprb/http#521)
  Skip auto-deflate when there is no body.
  ([@Bonias])

* [#489](httprb/http#489)
  Fix HTTP parser.
  ([@ixti], [@fxposter])

* [#546](httprb/http#546)
  **BREAKING CHANGE**
  Provide initiating `HTTP::Request` object on `HTTP::Response`.
  ([@joshuaflanagan])

* [#571](httprb/http#571)
  Drop Ruby 2.3.x support.
  ([@ixti])

* [3ed0c31](httprb/http@3ed0c31)
  Drop Ruby 2.4.x support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Time to drop http-parser?

5 participants

@bryanp @tarcieri @ixti @headius @midnight-wonderer