Skip to content

httpc:request_uri does not set ssl_server_name, breaking SNI #236

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

Closed
hackedd opened this issue Apr 9, 2021 · 10 comments · Fixed by #237
Closed

httpc:request_uri does not set ssl_server_name, breaking SNI #236

hackedd opened this issue Apr 9, 2021 · 10 comments · Fixed by #237

Comments

@hackedd
Copy link

hackedd commented Apr 9, 2021

In lua-resty-http 0.15.0, request_uri would call ssl_handshake with the hostname from the request. In 0.16.0 this is no longer the case, and to get SNI to work, you have to manually pass ssl_server_name.

@hackedd
Copy link
Author

hackedd commented Apr 9, 2021

I believe this could be fixed by letting ssl_server_name default to the parsed host, similar to path and query.

@pintsized
Copy link
Member

Yeah, makes sense to me. Good catch! @Tieske any reason why we shouldn't default to host in the absence of ssl_server_name?

@Tieske
Copy link
Contributor

Tieske commented Apr 9, 2021

should it only look at the host from the parsed url, or also at the host header if provided?

@Tieske
Copy link
Contributor

Tieske commented Apr 9, 2021

old implementation did not take the host header into account

@pintsized
Copy link
Member

Yeah, I think host from the parsed uri makes sense, if not only for backwards compatibility. The point of request_uri is to be a "simple" interface ultimately. And if you need to be specific, then we have ssl_server_name.

@Tieske
Copy link
Contributor

Tieske commented Apr 9, 2021

this comment is wrong:

Note also that fields in params will override relevant components of the uri if specified.

considering the 0.15 code: https://github.com/ledgetech/lua-resty-http/blob/v0.15/lib/resty/http.lua#L843-L845
(behaviour in 0.16 is same as 0.15)

Should be

Note also that fields path, and query, in params will override relevant components of the uri if specified (scheme, host, and port will always be taken from the uri).

@pintsized
Copy link
Member

Yeah right. Ok, I'll put a PR together to fix this up, unless you're already on the case?

Tieske added a commit to Tieske/lua-resty-http that referenced this issue Apr 9, 2021
also fixes an incorrect note in the docs

fixes ledgetech#236
@Tieske
Copy link
Contributor

Tieske commented Apr 9, 2021

hold on ...

@Tieske
Copy link
Contributor

Tieske commented Apr 9, 2021

there ^^

pintsized pushed a commit that referenced this issue Apr 9, 2021
also fixes an incorrect note in the docs

fixes #236
@pintsized
Copy link
Member

Merged and uploaded to luarocks. Thanks all.

Note opm releases are waiting on this opm fix to be merged and deployed before I can upload again :/

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