Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(84)

Unified Diff: third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp

Issue 1378543002: Add UMA for header values in XHR's setRequestHeader() checked against RFC 7230 (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Reflected tyoshino's comments. Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
diff --git a/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp b/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
index 7e5e8ef292b0cdce0a6fcd28d3c14b48ba903465..a996e3919cb1830b7f5d7710bafaa55fcf2f62b0 100644
--- a/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
+++ b/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
@@ -1174,10 +1174,6 @@ void XMLHttpRequest::setRequestHeader(const AtomicString& name, const AtomicStri
return;
}
- // Show deprecation warnings and count occurrences of such deprecated header values.
- if (!value.isEmpty() && !isValidHTTPFieldContentRFC7230(value))
- UseCounter::countDeprecation(executionContext(), UseCounter::HeaderValueNotMatchingRFC7230);
-
// No script (privileged or not) can set unsafe headers.
if (FetchUtils::isForbiddenHeaderName(name)) {
logConsoleError(executionContext(), "Refused to set unsafe header \"" + name + "\"");
@@ -1189,9 +1185,32 @@ void XMLHttpRequest::setRequestHeader(const AtomicString& name, const AtomicStri
void XMLHttpRequest::setRequestHeaderInternal(const AtomicString& name, const AtomicString& value)
{
+ // We show deprecation warnings if |value| is still invalid header value
+ // after normalization (i.e. contains invalid octets).
+ String normalizedValue = FetchUtils::normalizeHeaderValue(value);
+ if (!normalizedValue.isEmpty() && !isValidHTTPFieldContentRFC7230(normalizedValue))
+ UseCounter::countDeprecation(executionContext(), UseCounter::HeaderValueNotMatchingRFC7230);
+
HTTPHeaderMap::AddResult result = m_requestHeaders.add(name, value);
- if (!result.isNewEntry)
- result.storedValue->value = result.storedValue->value + ", " + value;
+ if (result.isNewEntry)
+ return;
+
+ AtomicString newValue = result.storedValue->value + ", " + value;
+
+ // We show deprecation warnings if this call to setRequestHeader() is
+ // affected by header value normalization.
+ // Without normalization at XHR level here, the actual header value
+ // sent to the network is |newValue| with leading/trailing whitespaces
+ // stripped (i.e. |normalizeHeaderValue(newValue)|).
+ // With normalization at XHR level here as the spec requires, the
+ // actual header value sent to the network is |normalizedNewValue|.
+ // If these two are different, introducing normalization here affects
+ // the header value sent to the network so we show warnings.
+ String normalizedNewValue = FetchUtils::normalizeHeaderValue(result.storedValue->value) + ", " + FetchUtils::normalizeHeaderValue(value);
+ if (FetchUtils::normalizeHeaderValue(newValue) != normalizedNewValue)
+ UseCounter::countDeprecation(executionContext(), UseCounter::XHRSetRequestHeaderAffectedByNormalization);
+
+ result.storedValue->value = newValue;
}
const AtomicString& XMLHttpRequest::getRequestHeader(const AtomicString& name) const

Powered by Google App Engine
This is Rietveld 408576698