Skip to content

Conversation

@hhorak
Copy link
Owner

@hhorak hhorak commented Mar 1, 2024

Original fix for v18:
nodejs@03a5c34

On top of the v18 patch, we needed to initialize the rest of callbacks in order to avoid warnings like

    ../src/node_http_parser.cc:967:1: warning: missing initializer for member
      'llhttp_settings_s::on_header_value_complete' [-Wmissing-field-initializers]

Replace llhttp_set_lenient with llhttp_set_lenient_*variants

This change is related to nodejs/llhttp@015e155 that replaced a single llhttp_set_lenient with more granular calls.
However, we only have lenient bool, so set it all or none.

hhorak added 2 commits March 1, 2024 13:17
Original fix for v18:
nodejs@03a5c34

On top of the v18 patch, we needed to initialize the rest of callbacks
in order to avoid warnings like
../src/node_http_parser.cc:967:1: warning: missing initializer for member
  'llhttp_settings_s::on_header_value_complete' [-Wmissing-field-initializers]
This change is related to nodejs/llhttp@015e155 that replaced a single llhttp_set_lenient with more granular calls.
However, we only have lenient bool, so set it all or none.
@richardlau
Copy link

I'm very wary about updating llhttp from 2.1.6 to 6.1.0 -- I think the failing tests indicates that there might be breaking changes there (maybe related to nodejs#38146 (comment)).

I've opened an alternative #3 where I've attempted to backport the feature used by nodejs@911cb33 onto the llhttp v2.1.x branch.

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 this pull request may close these issues.

3 participants