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

bugfix: fix build error with nginx >= 1.23.0 #134

Merged
merged 1 commit into from Jun 21, 2022
Merged

bugfix: fix build error with nginx >= 1.23.0 #134

merged 1 commit into from Jun 21, 2022

Conversation

somnisomni
Copy link
Contributor

@somnisomni somnisomni commented Jun 17, 2022

This will fix #132 .

cookies field name just renamed to cookie, according to nginx/nginx@3aef1d6 .

@somnisomni somnisomni changed the title bugfix: fixed build error with nginx >= 1.23.0 bugfix: fix build error with nginx >= 1.23.0 Jun 17, 2022
@eilandert
Copy link

please merge ;-)

@thibaultcha thibaultcha merged commit e536bc5 into openresty:master Jun 21, 2022
@RekGRpth
Copy link

test t/input-cookie.t TEST 4 seg faults!

@RekGRpth
Copy link

cookies field name just renamed to cookie

NOT! but Multi headers are now using linked lists instead of arrays!!!

@RekGRpth
Copy link

I suggest You should drop ngx_http_set_builtin_multi_header and replace it use by ngx_http_set_builtin_header

@somnisomni
Copy link
Contributor Author

somnisomni commented Jun 23, 2022

Okay maybe I missed something from nginx's commit. I think #136 fix this rest issue of this PR, but am I still have to follow @RekGRpth's suggestion:

I suggest You should drop ngx_http_set_builtin_multi_header and replace it use by ngx_http_set_builtin_header

?

@hnakamur
Copy link
Contributor

I ran t/bug.t with ngx_http_set_builtin_header for Cache-Control and with openresty/lua-nginx-module#2063

    { ngx_string("Cache-Control"),
                 offsetof(ngx_http_headers_out_t, cache_control),
                 ngx_http_set_builtin_header },

TEST 13 fails:

$ TEST_NGINX_BINARY=/path/to/my/nginx prove t/bug.t 
t/bug.t .. 1/112 
#   Failed test 't/bug.t TEST 13: set multi values to cache-control and override it with multiple values (to reproduce a bug) - header Cache-Control ok'
#   at /usr/local/share/perl/5.34.0/Test/Nginx/Socket.pm line 1010.
#          got: 'no-store, foo, bar, baz, blah'
#     expected: 'blah'

#   Failed test 't/bug.t TEST 13: set multi values to cache-control and override it with multiple values (to reproduce a bug) - response_body - response is expected (repeated req 0, req 0)'
#   at /usr/local/share/perl/5.34.0/Test/Nginx/Socket.pm line 1665.
#          got: "Cache-Control: no-store, foo, bar, baz, blah\x{0a}"
#       length: 45
#     expected: "Cache-Control: blah\x{0a}"
#       length: 20
#     strings begin to differ at char 16 (line 1 column 16)

#   Failed test 't/bug.t TEST 13: set multi values to cache-control and override it with multiple values (to reproduce a bug) - header Cache-Control ok'
#   at /usr/local/share/perl/5.34.0/Test/Nginx/Socket.pm line 1010.
#          got: 'no-store, foo, bar, baz, blah'
#     expected: 'blah'

#   Failed test 't/bug.t TEST 13: set multi values to cache-control and override it with multiple values (to reproduce a bug) - response_body - response is expected (repeated req 1, req 0)'
#   at /usr/local/share/perl/5.34.0/Test/Nginx/Socket.pm line 1665.
#          got: "Cache-Control: no-store, foo, bar, baz, blah\x{0a}"
#       length: 45
#     expected: "Cache-Control: blah\x{0a}"
#       length: 20
#     strings begin to differ at char 16 (line 1 column 16)
t/bug.t .. 107/112 # Looks like you failed 4 tests of 112.
t/bug.t .. Dubious, test returned 4 (wstat 1024, 0x400)
Failed 4/112 subtests 

Test Summary Report
-------------------
t/bug.t (Wstat: 1024 Tests: 112 Failed: 4)
  Failed tests:  8-9, 11-12
  Non-zero exit status: 4
Files=1, Tests=112,  4 wallclock secs ( 0.03 usr  0.01 sys +  0.36 cusr  0.15 csys =  0.55 CPU)
Result: FAIL

On the other hand, with #136 and openresty/lua-nginx-module#2063, all tests in t/bug.tpasses:

$ TEST_NGINX_BINARY=/path/to/my/nginx prove t/bug.t 
t/bug.t .. ok       
All tests successful.
Files=1, Tes

macbre added a commit to macbre/docker-nginx-http3 that referenced this pull request Jun 27, 2022
hkwi added a commit to hkwi/personium-ansible that referenced this pull request Apr 4, 2023
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.

headers-more-nginx-module not compatible with nginx 1.23.0
5 participants