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.

Sign up for GitHub

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

Jump to bottom

Use hostname in secure mode for the resolved service #375

Merged
merged 2 commits into from
Feb 23, 2024
Merged

Use hostname in secure mode for the resolved service #375

merged 2 commits into from
Feb 23, 2024

Conversation

lo-simon
Copy link
Contributor

  • host name should be used instead of the IP adddress for the host in the service uri.
  • the port number used in the Host header causes SSL handshake Error, disable it for now.

@garethsb
Copy link
Contributor

Thanks for debugging this! I think we can simplify this a bit further, I'll have a crack at it

@garethsb garethsb force-pushed the fix-uri branch 3 times, most recently from be297a5 to 7480e93 Compare February 23, 2024 10:21
garethsb and others added 2 commits February 23, 2024 12:27
@garethsb @lo-simon
In secure mode, don't append the port to the Host header
cbb6d0f 
because both calc_cn_host in cpprestsdk/Release/src/http/client/http_client_asio.cpp
and winhttp_client::send_request in cpprestsdk/Release/src/http/client/http_client_winhttp.cpp
compare the entire Host header value with the certificate Common Name
which causes an SSL handshake error

Co-authored-by: Simon Lo <[email protected]>
@garethsb
Fix missed use of nmos::details::make_http_client to unstash the host…
a2ac0cf 
… name for the Host header
@jonathan-r-thorpe jonathan-r-thorpe merged commit 49649f8 into sony:master Feb 23, 2024
10 of 11 checks passed
@lo-simon lo-simon deleted the fix-uri branch February 23, 2024 16:16
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.
3 participants
@lo-simon @garethsb @jonathan-r-thorpe