Skip to content

Fix: HTTPHdr host cache invalidation when Host header modified (#12768) #12796

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

Open
bryancall wants to merge 1 commit into
base: 9.2.x
Choose a base branch
from
+434 −2
Open

Fix: HTTPHdr host cache invalidation when Host header modified (#12768) #12796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions proxy/hdrs/HTTP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,14 +469,23 @@ bool is_http1_hdr_version_supported(const HTTPVersion &http_version);

class IOBufferReader;

/** HTTP Header class.
*
* @warning Changing the size of this class (adding/removing fields) will change
* the on-disk cache format and cause cache incompatibility. The HTTPCacheAlt
* structure contains embedded HTTPHdr objects, and the cache marshalling code
* uses sizeof(HTTPCacheAlt) to read/write cache entries. Any size change will
* cause "vector inconsistency" errors when reading cache entries written by a
* different version.
*/
class HTTPHdr : public MIMEHdr
{
public:
HTTPHdrImpl *m_http = nullptr;
// This is all cached data and so is mutable.
mutable URL m_url_cached;
mutable MIMEField *m_host_mime = nullptr;
mutable int m_host_length = 0; ///< Length of hostname.
mutable int m_host_length = 0; ///< Length of hostname (parsed, excludes port).
mutable int m_port = 0; ///< Target port.
mutable bool m_target_cached = false; ///< Whether host name and port are cached.
mutable bool m_target_in_url = false; ///< Whether host name and port are in the URL.
Expand Down Expand Up @@ -769,8 +778,34 @@ HTTPHdr::print(char *buf, int bufsize, int *bufindex, int *dumpoffset)
inline void
HTTPHdr::_test_and_fill_target_cache() const
{
if (!m_target_cached)
if (!m_target_cached) {
this->_fill_target_cache();
return;
}

// If host came from the Host header (not URL), check for staleness by verifying
// the current Host header value length matches what we expect from cached values.
if (!m_target_in_url && m_host_mime != nullptr) {
int expected_len = m_host_length;
if (m_port_in_header && m_port > 0) {
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

The condition m_port_in_header && m_port > 0 may not correctly handle all cases. If the Host header explicitly specifies port 0 (e.g., "example.com:0"), the condition will be false and won't add the port digits to expected_len, even though ":0" would be present in the header value. While port 0 is unusual, if it can be parsed and cached, the staleness check should handle it correctly. Consider whether m_port > 0 should be m_port >= 0 or if port 0 should be handled differently.

Suggested change
if (m_port_in_header && m_port > 0) {
if (m_port_in_header && m_port >= 0) {

Copilot uses AI. Check for mistakes.
// Account for ":port" suffix in the raw Host header value.
expected_len += 1; // colon
if (m_port < 10) {
expected_len += 1;
} else if (m_port < 100) {
expected_len += 2;
} else if (m_port < 1000) {
expected_len += 3;
} else if (m_port < 10000) {
expected_len += 4;
} else {
expected_len += 5;
}
}
if (m_host_mime->m_len_value != expected_len) {
this->_fill_target_cache();
}
}
Comment on lines +786 to +808
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

The cache invalidation logic has a subtle bug: it only checks if the total Host header value length has changed, but doesn't detect cases where the hostname length changes while the total length (hostname + port) stays the same. For example, if the Host header changes from "short.co:8080" (8+1+4=13 chars) to "longer.com:80" (10+1+2=13 chars), the cache won't be invalidated because expected_len equals m_host_mime->m_len_value (both 13). However, m_host_length (cached as 8) no longer matches the actual hostname length (10), causing host_get() to return incorrect results.

A more robust approach would be to also verify that the hostname portion hasn't changed, perhaps by checking if m_host_mime->m_ptr_value (up to m_host_length characters) still matches what we expect, or by storing a checksum of the hostname.

Copilot uses AI. Check for mistakes.
}

/*-------------------------------------------------------------------------
Expand Down
Loading