fix: Use vhost name instead of Host header in cache key calculation #2435
+273
−35
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53e6a3f to
8fc689e
Compare
| c = NULL; \ | ||
| if (!(u_fin = WARN_ON_ONCE(TFW_STR_EMPTY(uri_path)))) { \ | ||
|
|
There was a problem hiding this comment.
Broken \ aliment at the end of the string
| memset(&vhost_empty, 0, sizeof(vhost_empty)); | ||
| vhost_empty.name = empty_name; | ||
|
|
||
| TfwStr host_header = { .data = (void *)"some.host.com", .len = 13 }; |
There was a problem hiding this comment.
All variables should be declared at the beginning of the function. ISO C90 forbids mixed declarations and code
fw/t/unit/test_http_cache.c
TfwStr host_header = { .data = (void *)"some.host.com", .len = 13 };
test_req->host = host_header;
TfwStr uri_path = { .data = (void *)"/test", .len = 5 };
There was a problem hiding this comment.
ISO C90 forbids mixed declarations and code
| TfwStr host_header = { .data = (void *)"some.host.com", .len = 13 }; | ||
| test_req->host = host_header; | ||
|
|
||
| TfwStr uri_path = { .data = (void *)"/test", .len = 5 }; |
There was a problem hiding this comment.
ISO C90 forbids mixed declarations and code
| .data = (void *)"same.host.example.com", | ||
| .len = 21 | ||
| }; | ||
| test_req->host = host; |
There was a problem hiding this comment.
ISO C90 forbids mixed declarations and code
| .data = (void *)"/test/path", | ||
| .len = 10 | ||
| }; | ||
| test_req->uri_path = uri_path; |
There was a problem hiding this comment.
ISO C90 forbids mixed declarations and code
| key2 = tfw_http_req_key_calc(test_req); | ||
|
|
||
| /* Keys should be different because vhost names are different */ | ||
| EXPECT_NE(key1, key2); |
There was a problem hiding this comment.
This expect fails, when I run this test.
|
I also catch kernel warning at the string 2281 (cache.c) |
|
I also create brunch and add test. We should always write python tests. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Use vhost name instead of Host header in cache key calculation
Why
This change ensures proper cache operation when requests are redirected to different vhosts through HTTP chains.
Previously, the cache key was built using uri_path + host (from Host header), which caused incorrect cache behavior when a request with a Host header for one vhost was redirected to a different vhost via HTTP chains. This would lead to storing the cached response under a key that combined the URI with the original Host header, rather than the actual vhost that served the content.
With this change, the cache key is now correctly built using uri_path + vhost_name, ensuring that the cache operates properly even when HTTP chains redirect requests between different vhosts. The code falls back to using the Host header when a vhost is not yet assigned.
Links
2366