Bug 11611 - REGRESSION: No http referer header sent on XMLHttpRequest
Summary: REGRESSION: No http referer header sent on XMLHttpRequest
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2006-11-15 19:43 PST by David Richardson
Modified: 2007-01-04 03:42 PST (History)
1 user (show)

See Also:


Attachments
proposed fix (10.36 KB, patch)
2007-01-03 05:57 PST, Alexey Proskuryakov
darin: review-
Details | Formatted Diff | Diff
proposed fix (10.41 KB, patch)
2007-01-03 10:56 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Richardson 2006-11-15 19:43:05 PST
Webkit Build 17408 does not send an http referer header with an xmlhttprequest. Webkit Build 17405 and release Safari both do.
Comment 1 Alexey Proskuryakov 2006-11-15 21:35:20 PST
Confirmed with r17760.
Comment 2 Alexey Proskuryakov 2007-01-03 05:57:33 PST
Created attachment 12185 [details]
proposed fix

As of r18544, I could only reproduce this with async requests. One reason was that String::operator!() is used to check if a referer has been already set, and the default implementation of this operator doesn't work correctly:

    if (!hideReferrer && !request.httpReferrer())
        newRequest.setHTTPReferrer(fl->outgoingReferrer());

Another reason was that ResourceRequest->NSURLRequest conversion logic was a bit messed up in SubresourceLoader::create(). I rewrote this method to use ResourceRequest::nsURLRequest(). One thing I'm not quite sure about is a FIXME that I removed:

// FIXME: Because of <rdar://problem/4803505>, the method has to be set before the body.

It was added in r17294 without a test or much explanation, and seems to be obsolete.
Comment 3 Darin Adler 2007-01-03 09:47:58 PST
Radar 4803505 is a Foundation framework bug Anders filed stating that -[NSURLRequest setHTTPMethod:] after -[NSURLRequest setHTTPBody:] resets the body.
Comment 4 Alexey Proskuryakov 2007-01-03 09:52:35 PST
Thank you! So, it indeed looks obsolete already, since there's no longer a need to reverse this order. ResourceRequest.nsURLRequest() also sets the method first.
Comment 5 Darin Adler 2007-01-03 10:46:37 PST
Comment on attachment 12185 [details]
proposed fix

+    NSMutableURLRequest *newNSURLRequest = [newRequest.nsURLRequest() mutableCopy];
+
+    // FIXME: should this be in ResourceRequest::nsURLRequest()?
+    wkSupportsMultipartXMixedReplace(newNSURLRequest);
 
     RefPtr<SubresourceLoader> subloader(new SubresourceLoader(frame, client));
     if (!subloader->load(newNSURLRequest))
         return 0;
 
-    [newNSURLRequest release];
-

Looks like a storage leak here. newNSURLRequest is a copy and needs to be released.

Otherwise looks quite good!
Comment 6 Alexey Proskuryakov 2007-01-03 10:56:53 PST
Created attachment 12191 [details]
proposed fix

Oops! Fixed.
Comment 7 Darin Adler 2007-01-03 11:03:55 PST
Comment on attachment 12191 [details]
proposed fix

r=me

I think it's ever so slightly better to use a bool for the result of SubresourceLoader::load -- then you could have only one release.
Comment 8 Alexey Proskuryakov 2007-01-04 03:42:58 PST
Committed revision 18577.