Skip to content
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.

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

Fixed header cache for unknown values #11808

Merged
merged 3 commits into from
May 21, 2024

Conversation

gregw
Copy link
Contributor

@gregw gregw commented May 18, 2024

Avoid adding the unknown marker into the CACHE index. Issue introduced in #11661 fixing #11659

Avoid adding the unknown marker into the CACHE index. Issue introduced in #11661 fixing #11659
@gregw gregw added Bug For general bugs on Jetty side Sponsored This issue affects a user with a commercial support agreement labels May 18, 2024
@gregw gregw requested review from joakime and sbordet May 18, 2024 23:24
Avoid adding the unknown marker into the CACHE index. Issue introduced in #11661 fixing #11659
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

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

Change look ok beside maybe not caching the no-space-after-column case, but broke other tests.

@@ -167,7 +167,8 @@ public class HttpParser
for (HttpHeader h : HttpHeader.values())
{
HttpField httpField = new HttpField(h, UNMATCHED_VALUE);
map.put(httpField.toString(), httpField);
map.put(h + ": ", httpField);
map.put(h + ":", httpField);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to cache the no-space-after-column case?
It's bad formatting by the client, so it deserves less efficiency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it in because I initially write a test that was just looking up "Content-Type:" (no space) and was surprised by the miss. But I will remove for now to just fix the issue and we can think about other optimizations separately.

Avoid adding the unknown marker into the CACHE index. Issue introduced in #11661 fixing #11659
@gregw gregw requested a review from sbordet May 19, 2024 21:34
@gregw gregw merged commit a1b3acb into jetty-12.0.x May 21, 2024
7 of 9 checks passed
@gregw gregw deleted the fix/jetty-12.0.x/fix_header_cache branch May 21, 2024 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Sponsored This issue affects a user with a commercial support agreement
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants