Bug 16189 - XMLHttpRequest::setRequestHeader() should not set certain headers
Summary: XMLHttpRequest::setRequestHeader() should not set certain headers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Julien Chaffraix
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-29 05:18 PST by Julien Chaffraix
Modified: 2007-12-01 08:51 PST (History)
0 users

See Also:


Attachments
patch & testcase (5.47 KB, patch)
2007-11-29 07:07 PST, Julien Chaffraix
mjs: review-
Details | Formatted Diff | Diff
Patch updated with Maciej's comments (5.34 KB, patch)
2007-11-30 10:08 PST, Julien Chaffraix
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 2007-11-29 05:18:38 PST
The XMLHttpRequest working draft gives a list of headers that should not be set by setRequestHeader() for security reasons. Currently some of them are missing.

Mozilla checks for all of them except the ones starting with "Proxy-".

Patch & testcase will follow.
Comment 1 Julien Chaffraix 2007-11-29 07:07:32 PST
Created attachment 17591 [details]
patch & testcase

Add all the headers specified by the draft (even "Proxy-" headers).
Comment 2 Maciej Stachowiak 2007-11-29 18:11:34 PST
Looks great! Minor coding style issue:

+    static String proxyString;
     
     if (forbiddenHeaders.isEmpty()) {
         forbiddenHeaders.add("accept-charset");
         forbiddenHeaders.add("accept-encoding");
+        forbiddenHeaders.add("connection");
         forbiddenHeaders.add("content-length");
-        forbiddenHeaders.add("expect");
+        forbiddenHeaders.add("content-transfer-encoding");
         forbiddenHeaders.add("date");
+        forbiddenHeaders.add("expect");
         forbiddenHeaders.add("host");
         forbiddenHeaders.add("keep-alive");
         forbiddenHeaders.add("referer");
@@ -107,9 +110,11 @@ static bool canSetRequestHeader(const String& name)
         forbiddenHeaders.add("transfer-encoding");
         forbiddenHeaders.add("upgrade");
         forbiddenHeaders.add("via");
+
+        proxyString = String("proxy-");


You could just write    static String proxyString("proxy-"), it will still be initialized only once.

r- to consider style request, but I'll happily r+ with that revision.


Comment 3 Maciej Stachowiak 2007-11-29 18:12:37 PST
Comment on attachment 17591 [details]
patch & testcase

As stated above, r- for style issue.
Comment 4 Julien Chaffraix 2007-11-30 10:08:41 PST
Created attachment 17609 [details]
Patch updated with Maciej's comments

> You could just write    static String proxyString("proxy-"), it will still be
> initialized only once.

I did not know. Thanks for the info !
Comment 5 Mark Rowe (bdash) 2007-12-01 08:51:40 PST
Landed in r28301.