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

Quoted Request Log #12834 #12835

Merged
merged 2 commits into from
Mar 3, 2025
Merged

Quoted Request Log #12834 #12835

merged 2 commits into from
Mar 3, 2025

Conversation

gregw
Copy link
Contributor

@gregw gregw commented Feb 28, 2025

Fixes #12834 Quote request log values that need to be quoted
Track quotes in format string.

Quote values that need to be quoted
Track quotes in format string.
@gregw gregw requested review from lachlan-roberts and lorban and removed request for lachlan-roberts February 28, 2025 14:20
janbartel
janbartel previously approved these changes Feb 28, 2025
Copy link
Contributor

@janbartel janbartel left a comment

Choose a reason for hiding this comment

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

I think it looks fine, but you may want to get a review from someone more au fait with the request log format.

@gregw gregw requested a review from lachlan-roberts March 1, 2025 08:35
@gregw gregw requested a review from lachlan-roberts March 1, 2025 16:53
lorban
lorban previously approved these changes Mar 3, 2025
{
}

//-----------------------------------------------------------------------------------//
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's get rid of these separators, here and in the method above.


HttpTester.Response response = getResponse("""
GET / HTTP/1.0
User-Agent: bad"value
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to motify this test with User-Agent: bad\\"value, as if an application had already escaped the dquote.

The result is: "User-Agent: \"bad\\\\\\\"value\", \"bad\\\\\\\"value\"".

Is this expected? I would have expected the original value (perhaps quoted), which was already properly escaped.

Signed-off-by: Simone Bordet <[email protected]>
@sbordet sbordet dismissed stale reviews from lorban and janbartel via 81336ab March 3, 2025 10:33
@sbordet sbordet self-requested a review March 3, 2025 10:34
@joakime joakime added High Priority Bug For general bugs on Jetty side labels Mar 3, 2025
@gregw gregw merged commit 921a77e into jetty-12.0.x Mar 3, 2025
7 of 10 checks passed
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 High Priority
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Quotes should be escaped in request logs
6 participants