[webkit-reviews] review granted: [Bug 184493] Disallow responses when a response contains invalid header values : [Attachment 400724] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 2 09:51:32 PDT 2020
Darin Adler <darin at apple.com> has granted Rob Buis <rbuis at igalia.com>'s request
for review:
Bug 184493: Disallow responses when a response contains invalid header values
https://bugs.webkit.org/show_bug.cgi?id=184493
Attachment 400724: Patch
https://bugs.webkit.org/attachment.cgi?id=400724&action=review
--- Comment #34 from Darin Adler <darin at apple.com> ---
Comment on attachment 400724
--> https://bugs.webkit.org/attachment.cgi?id=400724
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=400724&action=review
> Source/WebCore/Modules/fetch/FetchHeaders.cpp:46
> + ASSERT(value.isEmpty() || (!isHTTPSpace(value[0]) &&
!isHTTPSpace(value[value.length() - 1])));
What guarantees what this asserts? Caller responsibility to call
stripLeadingAndTrailingHTTPSpaces? What's the benefit of this assertion? Seems
like the check will fail in this case, so it won’t "slip by"?
> Source/WebCore/Modules/fetch/FetchHeaders.cpp:81
> + String normalizedValue =
stripLeadingAndTrailingHTTPSpaces(header.value);
Seems unnecessarily inefficient to allocate a new String if there are leading
or trailing spaces. Cleaner if some day we could refactor to do these
operations with StringView instead, which would avoid memory allocation
entirely.
> Source/WebCore/Modules/fetch/FetchHeaders.cpp:212
> + String normalizedValue =
stripLeadingAndTrailingHTTPSpaces(header.value);
Ditto.
> Source/WebCore/platform/network/ResourceResponseBase.cpp:845
> + if
(!isValidHTTPHeaderValue(stripLeadingAndTrailingHTTPSpaces(header.value)))
Ditto.
More information about the webkit-reviews
mailing list