[webkit-reviews] review granted: [Bug 20792] Add origin header to POST requests : [Attachment 23744] updated patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 3 13:24:27 PDT 2008

Darin Adler <darin at apple.com> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 20792: Add origin header to POST requests

Attachment 23744: updated patch

------- Additional Comments from Darin Adler <darin at apple.com>
The biggest problem is that this sprinkles calls to a new function all over the
loader classes. I'm concerned that we have to add the addHTTPOriginIfNeeded
function call in so many places. It seems to me that will be hard to maintain
-- it will be very easy to introduce a security problem by forgetting to add a
call to that function. Is there any way to streamline this? I suppose we have
the same issue with the referrer, but it's not even true that these are all the
sites that set the referrer!

The loader is already not so great in this respect, but it seems this patch
makes it even worse.

+    RefPtr<SecurityOrigin> origin = SecurityOrigin::createEmpty();
+    return origin->toHTTPOrigin();

The local variable isn't needed and the code would read more cleanly without

+    // Make sure we send the Origin header properly.
+    addHTTPOriginIfNeeded(request, String());

I don't understand the meaning of the word "properly" in this comment.

+    // will leak the internal host name.  Similar privacy concerns have lead

+	 // header.  This is similar to toString(), except that the empty

We normally don't use two spaces between sentences in comments.

+	 RefPtr<SecurityOrigin> emptyOrigin = SecurityOrigin::createEmpty();
+	 origin = emptyOrigin->toHTTPOrigin();

The local variable isn't needed and the code would read more cleanly without
it. Maybe also a helper function named SecurityOrigin::emptyHTTPOrigin() would
be good?

+	 // SecurityOrigin is represetned as the string "null".

Typo: "represetned".

I'm going to say review+ despite my comments and minor concerns.

More information about the webkit-reviews mailing list