Skip to content

fix: strip hop-by-hop headers from bidirectional response#7

Merged
evgenizb merged 1 commit into
masterfrom
fix/strip-hop-by-hop-headers
Jun 3, 2026
Merged

fix: strip hop-by-hop headers from bidirectional response#7
evgenizb merged 1 commit into
masterfrom
fix/strip-hop-by-hop-headers

Conversation

@evgenizb

@evgenizb evgenizb commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Forwarding cm2's Transfer-Encoding/Connection/Keep-Alive headers back through the Lambda Function URL caused Step Functions HTTP Invoke to hang for 60s waiting for chunked-stream framing it does not support. Filter the RFC 7230 hop-by-hop set plus Content-Length/Content-Encoding before send_task_success.

Forwarding cm2's Transfer-Encoding/Connection/Keep-Alive headers back
through the Lambda Function URL caused Step Functions HTTP Invoke to
hang for 60s waiting for chunked-stream framing it does not support.
Filter the RFC 7230 hop-by-hop set plus Content-Length/Content-Encoding
before send_task_success.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to filter out hop-by-hop headers from proxy responses by defining a HOP_BY_HOP_HEADERS set and a filter_response_headers helper function. The feedback suggests adding the common 'proxy-connection' header to the filtered set to prevent potential connection issues, and adding a defensive check to handle cases where the input headers might be None or empty.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread cm_proxy/proxy.py
Comment on lines +307 to +320
HOP_BY_HOP_HEADERS = frozenset(
{
"connection",
"keep-alive",
"proxy-authenticate",
"proxy-authorization",
"te",
"trailer",
"transfer-encoding",
"upgrade",
"content-length",
"content-encoding",
}
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The non-standard but widely used Proxy-Connection header behaves similarly to the standard Connection header and is often sent by older proxies or clients. To prevent potential hanging or connection issues, it is safer to include "proxy-connection" in the set of hop-by-hop headers to be filtered out.

Suggested change
HOP_BY_HOP_HEADERS = frozenset(
{
"connection",
"keep-alive",
"proxy-authenticate",
"proxy-authorization",
"te",
"trailer",
"transfer-encoding",
"upgrade",
"content-length",
"content-encoding",
}
)
HOP_BY_HOP_HEADERS = frozenset(
{
"connection",
"keep-alive",
"proxy-authenticate",
"proxy-authorization",
"proxy-connection",
"te",
"trailer",
"transfer-encoding",
"upgrade",
"content-length",
"content-encoding",
}
)

Comment thread cm_proxy/proxy.py
Comment on lines +323 to +324
def filter_response_headers(headers):
return {k: v for k, v in headers.items() if k.lower() not in HOP_BY_HOP_HEADERS}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To ensure robustness and adhere to defensive programming practices, it is safer to handle cases where headers might be None or empty (for example, in unit tests or mock environments) by returning an empty dictionary early.

Suggested change
def filter_response_headers(headers):
return {k: v for k, v in headers.items() if k.lower() not in HOP_BY_HOP_HEADERS}
def filter_response_headers(headers):
if not headers:
return {}
return {k: v for k, v in headers.items() if k.lower() not in HOP_BY_HOP_HEADERS}

@evgenizb evgenizb merged commit 3501508 into master Jun 3, 2026
@evgenizb evgenizb deleted the fix/strip-hop-by-hop-headers branch June 3, 2026 11:03
@evgenizb evgenizb mentioned this pull request Jun 3, 2026
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