Log URL with correct protocol in Net::HTTP requests#186
Log URL with correct protocol in Net::HTTP requests#186amleaver wants to merge 1 commit intotrusche:masterfrom
Conversation
Previously URLs were always logged with "http://" even if the connection was made over SSL.
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where URLs were always logged with "http://" prefix even when connections were made over SSL. The fix checks the use_ssl? method to determine whether to use "https" or "http" as the protocol prefix.
Changes:
- Added a private
protocolhelper method that returns "https" or "http" based on the SSL connection state - Updated URL construction to use the dynamic protocol instead of hardcoded "http://"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def protocol | ||
| use_ssl? ? 'https' : 'http' | ||
| end |
There was a problem hiding this comment.
While the implementation is correct, there is currently no test coverage for HTTPS connections in the test suite. The shared examples in spec/support/shared_examples.rb:3 hardcode "http://" in the expected URL, and none of the test adapters create SSL connections. Consider adding test coverage for HTTPS scenarios to ensure this change works as expected and to prevent regressions.
Previously URLs were always logged with "http://" even if the connection was made over SSL. E.g.
use_ssl?is now checked when deciding what protocol to prefix the URL with.Test are passing and tweak has been manually confirmed with an SSL connection: