Move method and header checking to AssociatedURLLoader, so we can reuse code in XMLHttpRequest. Add an 'untrustedHttp' option to WebURLLoaderOptions, and if it's set, asynchronously return an error if the HTTP request is unsafe.
Created attachment 105224 [details] Proposed Patch
Comment on attachment 105224 [details] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=105224&action=review > Source/WebKit/chromium/public/WebURLLoaderOptions.h:2 > + * Copyright (C) 2011 Google Inc. All rights reserved. In WebKit, we typically don't throw away years like this (but whatever). :) > Source/WebKit/chromium/public/WebURLLoaderOptions.h:45 > + : untrustedHttp(false) Need 4 space indent. > Source/WebKit/chromium/public/WebURLLoaderOptions.h:49 > + , crossOriginRequestPolicy(CrossOriginRequestPolicyDeny) { } Put {} on new lines. > Source/WebKit/chromium/src/AssociatedURLLoader.cpp:73 > + m_isSafe = false; Alternately m_isSafe = m_isSafe && XMLHttpRequest::isSafeRequestHeader(name) && XMLHttpRequest::isValidHeaderValue(value); Also why isn't there a XMLHttpRequest::isValidToken(name) check here? > Source/WebKit/chromium/src/AssociatedURLLoader.cpp:284 > m_clientAdapter->enableErrorNotifications(); Does the didFail go through since the enableErrorNotifications happens here? > Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp:214 > + EXPECT_TRUE(m_didFail); Lines 204-214 seems like that repeat lines 184-194. Can we create a common function instead? Even the first three lines are dups and feel like they should also be a common function. > Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp:343 > +} No tests for invalid header values or headers names with invalid tokens.
Comment on attachment 105224 [details] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=105224&action=review >> Source/WebKit/chromium/public/WebURLLoaderOptions.h:45 >> + : untrustedHttp(false) > > Need 4 space indent. nit: untrustedHttp -> untrustedHTTP see webkit style guide where it mentions capitalization rules for acronyms. > Source/WebKit/chromium/src/AssociatedURLLoader.cpp:58 > +class SafeHttpHeaderValidator : public WebHTTPHeaderVisitor { nit: SafeHttp -> SafeHTTP > Source/WebKit/chromium/src/AssociatedURLLoader.cpp:282 > + m_clientAdapter->didFail(ResourceError()); maybe we should have a setDelayedError method to make it more explicit what we are doing here?
*** This bug has been marked as a duplicate of bug 67655 ***