-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix(storage): use regex in v4SanitizeHeaders #6970
Conversation
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
@@ -535,13 +535,12 @@ func v4SanitizeHeaders(hdrs []string) []string { | |||
sanitizedHeader := strings.TrimSpace(hdr) | |||
|
|||
var key, value string | |||
headerMatches := strings.Split(sanitizedHeader, ":") | |||
if len(headerMatches) < 2 { | |||
headerMatches := canonicalHeaderRegexp.FindStringSubmatch(sanitizedHeader) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think not using the canonicalHeaderRegexp is actually an intentional difference between v2 and v4; see comments in L526-527.
However, maybe we should use strings.SplitN instead so that only the first :
is split on. Does that sound like a good solution to you?
Closing this as #7603 contains the intended changes. As of release v1.30.1, storage allows the use of values containing |
The
v4SanitizeHeaders()
logic usedsplit(":")
func to separate the headerskey:val
, but in cases where the value itself contains:
(such as dates inRFC3339 format
as required), the value is partly lost.I replaced the logic with a regex logic- same as the one used in
v2SanitizeHeaders()
and added a unit test for this case.