Skip to content

Commit f70bfc7

Browse files
annevkannevk
committed
Merge isXMLSpace() into isJSONOrHTTPWhitespace()
https://bugs.webkit.org/show_bug.cgi?id=255872 rdar://108738795 Reviewed by Darin Adler. It turns out that JSON, HTTP, and XML all use the same whitespace definition, so let's make them share it. Also correct an existing comment for that function as \v is not part of isASCIIWhitespace(), but \f is. Furthermore, remove the "optimization" from these whitespace functions per a comment from Chris Dumez at #13080 (comment): > Just verified out of curiosity and llvm does generate the same code > with -O2 (tried both arm64 and x86_64): > > isXMLSpace1(char): // @isXMLSpace1(char) > mov x8, #9728 // =0x2600 > and w9, w0, #0xff > movk x8, #1, lsl #32 > cmp w9, #33 > cset w9, lo > lsr x8, x8, x0 > and w0, w9, w8 > ret > isXMLSpace2(char): // @isXMLSpace2(char) > mov x8, #9728 // =0x2600 > and w9, w0, #0xff > movk x8, #1, lsl #32 > cmp w9, #33 > cset w9, lo > lsr x8, x8, x0 > and w0, w9, w8 > ret > > Ahmad-S792 Let's simplify the code then. * Source/WTF/wtf/ASCIICType.h: (WTF::isASCIIWhitespace): (WTF::isJSONOrHTTPOrXMLWhitespace): (WTF::isJSONOrHTTPWhitespace): Deleted. * Source/WTF/wtf/JSONValues.cpp: (WTF::JSONImpl::Value::parseJSON): * Source/WTF/wtf/text/StringToIntegerConversion.h: * Source/WebCore/Modules/cache/DOMCache.cpp: (WebCore::hasResponseVaryStarHeaderValue): * Source/WebCore/Modules/cache/DOMCacheEngine.cpp: (WebCore::DOMCacheEngine::queryCacheMatch): * Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp: (WebCore::parseParameters): (WebCore::parseMIMEType): (WebCore::FetchBodyConsumer::packageFormData): * Source/WebCore/Modules/fetch/FetchHeaders.cpp: (WebCore::canWriteHeader): (WebCore::appendToHeaderMap): (WebCore::FetchHeaders::set): (WebCore::FetchHeaders::filterAndFill): * Source/WebCore/mathml/MathMLPresentationElement.cpp: (WebCore::MathMLPresentationElement::parseMathMLLength): * Source/WebCore/mathml/MathMLTokenElement.cpp: (WebCore::MathMLTokenElement::convertToSingleCodePoint): * Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp: (WebCore::ContentSecurityPolicyDirectiveList::parse): * Source/WebCore/platform/ReferrerPolicy.cpp: (WebCore::parseReferrerPolicy): * Source/WebCore/platform/network/DataURLDecoder.cpp: (WebCore::DataURLDecoder::DecodeTask::process): * Source/WebCore/platform/network/HTTPParsers.cpp: (WebCore::parseContentTypeOptionsHeader): (WebCore::parseClearSiteDataHeader): (WebCore::parseRange): (WebCore::parseCrossOriginResourcePolicyHeader): * Source/WebCore/platform/network/HTTPParsers.h: (WebCore::addToAccessControlAllowList): * Source/WebCore/platform/network/ParsedContentType.cpp: (WebCore::skipSpaces): (WebCore::parseToken): (WebCore::ParsedContentType::create): (WebCore::ParsedContentType::setContentType): * Source/WebCore/platform/network/ResourceResponseBase.cpp: (WebCore::ResourceResponseBase::containsInvalidHTTPHeaders const): * Source/WebCore/platform/network/TimingAllowOrigin.cpp: (WebCore::passesTimingAllowOriginCheck): * Source/WebCore/xml/XMLHttpRequest.cpp: (WebCore::XMLHttpRequest::setRequestHeader): * Source/WebCore/xml/XPathFunctions.cpp: (WebCore::XPath::FunNormalizeSpace::evaluate const): * Source/WebCore/xml/XPathParser.cpp: (WebCore::XPath::Parser::skipWS): * Source/WebCore/xml/XPathUtil.cpp: (WebCore::XPath::isXMLSpace): Deleted. * Source/WebCore/xml/XPathUtil.h: * Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp: (WebKit::CacheStorage::updateVaryInformation): * Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp: (WebKit::WebSocketTask::WebSocketTask): * Source/WebKit/NetworkProcess/storage/CacheStorageRecord.h: (WebKit::CacheStorageRecordInformation::updateVaryHeaders): * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::shouldSkipDecidePolicyForResponse const): Canonical link: https://commits.webkit.org/266253@main
1 parent cc626f3 commit f70bfc7

26 files changed

+50
-81
lines changed

‎Source/WTF/wtf/ASCIICType.h‎

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -141,46 +141,24 @@ template<typename CharacterType> constexpr bool isTabOrSpace(CharacterType chara
141141
// Infra's "ASCII whitespace" <https://infra.spec.whatwg.org/#ascii-whitespace>
142142
template<typename CharacterType> constexpr bool isASCIIWhitespace(CharacterType character)
143143
{
144-
// Histogram from Apple's page load test combined with some ad hoc browsing some other test suites.
145-
//
146-
// 82%: 216330 non-space characters, all > U+0020
147-
// 11%: 30017 plain space characters, U+0020
148-
// 5%: 12099 newline characters, U+000A
149-
// 2%: 5346 tab characters, U+0009
150-
//
151-
// No other characters seen. No U+000C or U+000D, and no other control characters.
152-
// Accordingly, we check for non-spaces first, then space, then newline, then tab, then the other characters.
153-
154-
return character <= ' ' && (character == ' ' || character == '\n' || character == '\t' || character == '\r' || character == '\f');
144+
return character == ' ' || character == '\n' || character == '\t' || character == '\r' || character == '\f';
155145
}
156146

157-
template<typename CharacterType> constexpr bool isJSONOrHTTPWhitespace(CharacterType character)
147+
template<typename CharacterType> constexpr bool isASCIIWhitespaceWithoutFF(CharacterType character)
158148
{
159-
// This is different from isASCIIWhitespace: JSON does not accept \v as a space.
160-
// ECMA-404 specifies the followings.
149+
// This is different from isASCIIWhitespace: JSON/HTTP/XML do not accept \f as a whitespace.
150+
// ECMA-404 specifies the following:
161151
// > Whitespace is any sequence of one or more of the following code points:
162152
// > character tabulation (U+0009), line feed (U+000A), carriage return (U+000D), and space (U+0020).
163153
//
164-
// Also, this definition is the same to HTTP whitespace.
154+
// This matches HTTP whitespace:
165155
// https://fetch.spec.whatwg.org/#http-whitespace-byte
166-
return character <= ' ' && (character == ' ' || character == '\n' || character == '\t' || character == '\r');
156+
//
157+
// And XML whitespace:
158+
// https://www.w3.org/TR/2008/REC-xml-20081126/#NT-S
159+
return character == ' ' || character == '\n' || character == '\t' || character == '\r';
167160
}
168161

169-
/*
170-
Statistics from a run of Apple's page load test for callers of isUnicodeCompatibleASCIIWhitespace:
171-
172-
character count
173-
--------- -----
174-
non-spaces 689383
175-
20 space 294720
176-
0A \n 89059
177-
09 \t 28320
178-
0D \r 0
179-
0C \f 0
180-
0B \v 0
181-
182-
As these are compatible with those for isASCIIWhitespace we let that do most of the work.
183-
*/
184162
template<typename CharacterType> constexpr bool isUnicodeCompatibleASCIIWhitespace(CharacterType character)
185163
{
186164
return isASCIIWhitespace(character) || character == '\v';
@@ -288,7 +266,7 @@ using WTF::isASCIIOctalDigit;
288266
using WTF::isASCIIPrintable;
289267
using WTF::isTabOrSpace;
290268
using WTF::isASCIIWhitespace;
291-
using WTF::isJSONOrHTTPWhitespace;
269+
using WTF::isASCIIWhitespaceWithoutFF;
292270
using WTF::isUnicodeCompatibleASCIIWhitespace;
293271
using WTF::isASCIIUpper;
294272
using WTF::isNotASCIIWhitespace;

‎Source/WTF/wtf/JSONValues.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ bool parseStringToken(const CodeUnit* start, const CodeUnit* end, const CodeUnit
207207
template<typename CodeUnit>
208208
Token parseToken(const CodeUnit* start, const CodeUnit* end, const CodeUnit** tokenStart, const CodeUnit** tokenEnd)
209209
{
210-
while (start < end && isJSONOrHTTPWhitespace(*start))
210+
while (start < end && isASCIIWhitespaceWithoutFF(*start))
211211
++start;
212212

213213
if (start == end)
@@ -524,7 +524,7 @@ RefPtr<Value> Value::parseJSON(StringView json)
524524
if (!begin)
525525
return false;
526526
for (const auto* it = begin; it < end; it++) {
527-
if (!isJSONOrHTTPWhitespace(*it))
527+
if (!isASCIIWhitespaceWithoutFF(*it))
528528
return true;
529529
}
530530
return false;

‎Source/WTF/wtf/text/StringToIntegerConversion.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace WTF {
3434
// The parseIntegerAllowingTrailingJunk function template is like parseInteger, but allows any characters after the integer.
3535

3636
// FIXME: Should we add a version that does not allow "+"?
37-
// FIXME: Should we add a version that allows other definitions of spaces, like isASCIIWhitespace or isJSONOrHTTPWhitespace?
37+
// FIXME: Should we add a version that allows other definitions of spaces, like isASCIIWhitespace or isASCIIWhitespaceWithoutFF?
3838
// FIXME: Should we add a version that does not allow leading and trailing spaces?
3939

4040
template<typename IntegralType> std::optional<IntegralType> parseInteger(StringView, uint8_t base = 10);

‎Source/WebCore/Modules/cache/DOMCache.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ static inline bool hasResponseVaryStarHeaderValue(const FetchResponse& response)
174174
auto varyValue = response.headers().internalHeaders().get(WebCore::HTTPHeaderName::Vary);
175175
bool hasStar = false;
176176
varyValue.split(',', [&](StringView view) {
177-
if (!hasStar && view.trim(isJSONOrHTTPWhitespace<UChar>) == "*"_s)
177+
if (!hasStar && view.trim(isASCIIWhitespaceWithoutFF<UChar>) == "*"_s)
178178
hasStar = true;
179179
});
180180
return hasStar;

‎Source/WebCore/Modules/cache/DOMCacheEngine.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ bool queryCacheMatch(const ResourceRequest& request, const ResourceRequest& cach
9797
varyValue.split(',', [&](StringView view) {
9898
if (isVarying)
9999
return;
100-
auto nameView = view.trim(isJSONOrHTTPWhitespace<UChar>);
100+
auto nameView = view.trim(isASCIIWhitespaceWithoutFF<UChar>);
101101
if (nameView == "*"_s) {
102102
isVarying = true;
103103
return;

‎Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ static HashMap<String, String> parseParameters(StringView input, size_t position
9595
size_t valueBegin = position;
9696
while (position < input.length() && input[position] != ';')
9797
position++;
98-
parameterValue = input.substring(valueBegin, position - valueBegin).trim(isJSONOrHTTPWhitespace<UChar>);
98+
parameterValue = input.substring(valueBegin, position - valueBegin).trim(isASCIIWhitespaceWithoutFF<UChar>);
9999
}
100100

101101
if (parameterName.length()
@@ -110,7 +110,7 @@ static HashMap<String, String> parseParameters(StringView input, size_t position
110110
// https://mimesniff.spec.whatwg.org/#parsing-a-mime-type
111111
static std::optional<MimeType> parseMIMEType(const String& contentType)
112112
{
113-
String input = contentType.trim(isJSONOrHTTPWhitespace<UChar>);
113+
String input = contentType.trim(isASCIIWhitespaceWithoutFF<UChar>);
114114
size_t slashIndex = input.find('/');
115115
if (slashIndex == notFound)
116116
return std::nullopt;
@@ -120,7 +120,7 @@ static std::optional<MimeType> parseMIMEType(const String& contentType)
120120
return std::nullopt;
121121

122122
size_t semicolonIndex = input.find(';', slashIndex);
123-
String subtype = input.substring(slashIndex + 1, semicolonIndex - slashIndex - 1).trim(isJSONOrHTTPWhitespace<UChar>);
123+
String subtype = input.substring(slashIndex + 1, semicolonIndex - slashIndex - 1).trim(isASCIIWhitespaceWithoutFF<UChar>);
124124
if (!subtype.length() || !isValidHTTPToken(subtype))
125125
return std::nullopt;
126126

@@ -175,7 +175,7 @@ RefPtr<DOMFormData> FetchBodyConsumer::packageFormData(ScriptExecutionContext* c
175175
size_t contentTypeBegin = header.find(contentTypeCharacters);
176176
if (contentTypeBegin != notFound) {
177177
size_t contentTypeEnd = header.find("\r\n"_s, contentTypeBegin);
178-
contentType = StringView(header).substring(contentTypeBegin + contentTypePrefixLength, contentTypeEnd - contentTypeBegin - contentTypePrefixLength).trim(isJSONOrHTTPWhitespace<UChar>).toString();
178+
contentType = StringView(header).substring(contentTypeBegin + contentTypePrefixLength, contentTypeEnd - contentTypeBegin - contentTypePrefixLength).trim(isASCIIWhitespaceWithoutFF<UChar>).toString();
179179
}
180180

181181
form.append(name, File::create(context, Blob::create(context, Vector { bodyBegin, bodyLength }, Blob::normalizedContentType(contentType)).get(), filename).get(), filename);

‎Source/WebCore/Modules/fetch/FetchHeaders.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ static ExceptionOr<bool> canWriteHeader(const String& name, const String& value,
4343
{
4444
if (!isValidHTTPToken(name))
4545
return Exception { TypeError, makeString("Invalid header name: '", name, "'") };
46-
ASSERT(value.isEmpty() || (!isJSONOrHTTPWhitespace(value[0]) && !isJSONOrHTTPWhitespace(value[value.length() - 1])));
46+
ASSERT(value.isEmpty() || (!isASCIIWhitespaceWithoutFF(value[0]) && !isASCIIWhitespaceWithoutFF(value[value.length() - 1])));
4747
if (!isValidHTTPHeaderValue((value)))
4848
return Exception { TypeError, makeString("Header '", name, "' has invalid value: '", value, "'") };
4949
if (guard == FetchHeaders::Guard::Immutable)
@@ -72,7 +72,7 @@ static ExceptionOr<void> appendSetCookie(const String& value, Vector<String>& se
7272

7373
static ExceptionOr<void> appendToHeaderMap(const String& name, const String& value, HTTPHeaderMap& headers, Vector<String>& setCookieValues, FetchHeaders::Guard guard)
7474
{
75-
String normalizedValue = value.trim(isJSONOrHTTPWhitespace<UChar>);
75+
String normalizedValue = value.trim(isASCIIWhitespaceWithoutFF<UChar>);
7676
if (equalIgnoringASCIICase(name, "set-cookie"_s))
7777
return appendSetCookie(normalizedValue, setCookieValues, guard);
7878

@@ -95,7 +95,7 @@ static ExceptionOr<void> appendToHeaderMap(const String& name, const String& val
9595
static ExceptionOr<void> appendToHeaderMap(const HTTPHeaderMap::HTTPHeaderMapConstIterator::KeyValue& header, HTTPHeaderMap& headers, FetchHeaders::Guard guard)
9696
{
9797
ASSERT(!equalIgnoringASCIICase(header.key, "set-cookie"_s));
98-
String normalizedValue = header.value.trim(isJSONOrHTTPWhitespace<UChar>);
98+
String normalizedValue = header.value.trim(isASCIIWhitespaceWithoutFF<UChar>);
9999
auto canWriteResult = canWriteHeader(header.key, normalizedValue, header.value, guard);
100100
if (canWriteResult.hasException())
101101
return canWriteResult.releaseException();
@@ -239,7 +239,7 @@ ExceptionOr<bool> FetchHeaders::has(const String& name) const
239239

240240
ExceptionOr<void> FetchHeaders::set(const String& name, const String& value)
241241
{
242-
String normalizedValue = value.trim(isJSONOrHTTPWhitespace<UChar>);
242+
String normalizedValue = value.trim(isASCIIWhitespaceWithoutFF<UChar>);
243243
auto canWriteResult = canWriteHeader(name, normalizedValue, normalizedValue, m_guard);
244244
if (canWriteResult.hasException())
245245
return canWriteResult.releaseException();
@@ -262,7 +262,7 @@ ExceptionOr<void> FetchHeaders::set(const String& name, const String& value)
262262
void FetchHeaders::filterAndFill(const HTTPHeaderMap& headers, Guard guard)
263263
{
264264
for (auto& header : headers) {
265-
String normalizedValue = header.value.trim(isJSONOrHTTPWhitespace<UChar>);
265+
String normalizedValue = header.value.trim(isASCIIWhitespaceWithoutFF<UChar>);
266266
auto canWriteResult = canWriteHeader(header.key, normalizedValue, header.value, guard);
267267
if (canWriteResult.hasException())
268268
continue;

‎Source/WebCore/mathml/MathMLPresentationElement.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ MathMLElement::Length MathMLPresentationElement::parseMathMLLength(const String&
183183

184184
// We first skip whitespace from both ends of the string.
185185
StringView stringView = string;
186-
StringView trimmedLength = stringView.trim(isJSONOrHTTPWhitespace<UChar>);
186+
StringView trimmedLength = stringView.trim(isASCIIWhitespaceWithoutFF<UChar>);
187187

188188
if (trimmedLength.isEmpty())
189189
return Length();

‎Source/WebCore/mathml/MathMLTokenElement.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ bool MathMLTokenElement::childShouldCreateRenderer(const Node& child) const
8282

8383
std::optional<UChar32> MathMLTokenElement::convertToSingleCodePoint(StringView string)
8484
{
85-
auto codePoints = string.trim(isJSONOrHTTPWhitespace<UChar>).codePoints();
85+
auto codePoints = string.trim(isASCIIWhitespaceWithoutFF<UChar>).codePoints();
8686
auto iterator = codePoints.begin();
8787
if (iterator == codePoints.end())
8888
return std::nullopt;

‎Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ void ContentSecurityPolicyDirectiveList::parse(const String& policy, ContentSecu
449449
// A meta tag delievered CSP could contain invalid HTTP header values depending on how it was formatted in the document.
450450
// We want to store the CSP as a valid HTTP header for e.g. blob URL inheritance.
451451
if (policyFrom == ContentSecurityPolicy::PolicyFrom::HTTPEquivMeta) {
452-
m_header = policy.trim(isJSONOrHTTPWhitespace<UChar>).removeCharacters([](auto c) {
452+
m_header = policy.trim(isASCIIWhitespaceWithoutFF<UChar>).removeCharacters([](auto c) {
453453
return c == 0x00 || c == '\r' || c == '\n';
454454
});
455455
} else

0 commit comments

Comments
 (0)