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

[bugzilla-daemon at webkit.org]

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
https://bugs.webkit.org/show_bug.cgi?id=20792

Attachment 23744: updated patch
https://bugs.webkit.org/attachment.cgi?id=23744&action=edit

------- 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
it.

+    // 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.