[Webkit-unassigned] [Bug 66963] [Chromium] Move untrusted HTTP method/header checking to AssociatedURLLoader.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=66963


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #105224|review?                     |review-
               Flag|                            |




--- Comment #2 from David Levin <levin at chromium.org>  2011-08-25 12:15:55 PST ---
(From update of attachment 105224)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.