Skip to content

Conversation

SamRemis
Copy link
Contributor

@SamRemis SamRemis commented Oct 17, 2025

This PR includes a bugfix which breaks signing any requests that are sent to a non-default port.

Problem:

There's a mismatch between the host header used for signing vs. the actual request header when connecting to non-default ports.

During signing: The host header includes port (from URI netloc) and gets added to signed headers. The header does not get added to the final request.
During request creation: The host header excludes port (from URI host) if not already present.
Result: The request is signed with example.com:8080 but sent with the host header having a value of example.com. This creates a signature validation issue server-side.

Solution:

This PR fixes the issue in two places:

  1. Signer: Ensures the host header added during signing is included in the actual request
  2. CRT client: Updates the HTTP client code to use netloc (with port) instead of host when adding the header

This ensures the host header value is consistent between signing and transmission for non-default ports.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@SamRemis SamRemis requested a review from a team as a code owner October 17, 2025 19:49
)

signing_fields = self._normalize_signing_fields(request=new_request)
# If any headers have been added, make sure those are reflected in the new request
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we can't apply this in the _apply_required_fields method where we set X-Amz-Date and X-Amz-Security-Token? Just seeing that now

if "host" not in request.fields:
request.fields.set_field(
Field(name="host", values=[request.destination.host])
Field(name="host", values=[request.destination.netloc])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should look into if netloc is the right choice. It would follow the format of {username}:{password}@{host}:{port}.

I see ruby does {host}:{port} (src)

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.

2 participants