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

Pass nil to Nginx::Headers_in[]= causes segmentaion fault #473

Closed
yano3 opened this issue Aug 17, 2020 · 5 comments · Fixed by #475
Closed

Pass nil to Nginx::Headers_in[]= causes segmentaion fault #473

yano3 opened this issue Aug 17, 2020 · 5 comments · Fixed by #475

Comments

@yano3
Copy link
Contributor

yano3 commented Aug 17, 2020

Pass nil to Nginx::Headers_in[]= causes segmentaion fault.

Reproduce

server {
    listen       80;

    location / {
        mruby_content_handler_code '
          h = Nginx::Headers_in.new
          Nginx.echo h["X-FOO"]
          h["X-FOO"] = nil
        ';
    }
}
$ curl -H "X-FOO: foo" http://localhost:10080/
curl: (52) Empty reply from server

The following error is output to error.log.

2020/08/17 07:34:24 [alert] 1#1: worker process 27 exited on signal 11

This also happen with Nginx::Headers_out[]= too.

@yyamano
Copy link
Collaborator

yyamano commented Aug 17, 2020

@yano3 Thank you for the report. What is the expected behavior? Is throwing an exception fine?

@yano3
Copy link
Contributor Author

yano3 commented Aug 17, 2020

In my use case, there is no problem with the behavior of throwing an exception. I only expect to avoid segmentation fault.

IMO, following behavior is good.

  • If specified header exists, delete it.
  • If specified header does not exist, do nothing.

For example, it's natural in the following case.

h = Nginx::Headers_in.new

# get header that does not exist
foo = h["X-FOO"] # => nil
h["X-FOO"] = foo

@yyamano
Copy link
Collaborator

yyamano commented Aug 18, 2020

Sounds good. I just sent a pull request #475

@yano3
Copy link
Contributor Author

yano3 commented Aug 18, 2020

Thank you!

@matsumotory
Copy link
Owner

@yano3 @yyamano Thank you for your proposal!

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