Bug 11611 – REGRESSION: No http referer header sent on XMLHttpRequest

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[David Richardson]

Webkit Build 17408 does not send an http referer header with an xmlhttprequest. Webkit Build 17405 and release Safari both do.

[Alexey Proskuryakov]

Confirmed with r17760.

[Alexey Proskuryakov]

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.

[Darin Adler]

Radar 4803505 is a Foundation framework bug Anders filed stating that -[NSURLRequest setHTTPMethod:] after -[NSURLRequest setHTTPBody:] resets the body.

[Alexey Proskuryakov]

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.

[Darin Adler]

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!

[Alexey Proskuryakov]

Created attachment 12191 [details]
proposed fix

Oops! Fixed.

[Darin Adler]

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.

[Alexey Proskuryakov]

Committed revision 18577.