Add option to suppress HTTP Referer header.

Add option to suppress HTTP Referer header.

BUG=none
TEST=start chrome and run tcpdump -A. Should be contain any referer header.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=38587

[jochen (gone - plz use gerrit), 2010-02-09 22:35:24]

please review.

[Peter Kasting, 2010-02-09 22:39:47]

This is b/2379855, isn't it?

LGTM

http://codereview.chromium.org/600008/diff/1/2
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/600008/diff/1/2#newcode704
chrome/browser/renderer_host/resource_dispatcher_host.cc:704:
request->set_referrer("");
Nit: "" -> std::string()

Also you can make this simpler:

request->set_referrer(CommandLine::ForCurrentProcess()->HasSwitch(
    switches::kNoReferrer) ? std::string : referrer.spec());

http://codereview.chromium.org/600008/diff/1/2#newcode762
chrome/browser/renderer_host/resource_dispatcher_host.cc:762: if
(CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoReferrer))
Same comments as above

http://codereview.chromium.org/600008/diff/1/3
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/600008/diff/1/3#newcode441
chrome/common/chrome_switches.cc:441: const char kNoReferrer[]                  
 = "no-referrer";
I think "no-referrers" (and pluralizing the switch name) might be a tiny bit
clearer.

[ian fette, 2010-02-09 23:11:04]

LGTM

http://codereview.chromium.org/600008/diff/1/2
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/600008/diff/1/2#newcode465
chrome/browser/renderer_host/resource_dispatcher_host.cc:465:
request->set_referrer("");
std::string() instead of "" as Peter points out below

http://codereview.chromium.org/600008/diff/1/3
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/600008/diff/1/3#newcode440
chrome/common/chrome_switches.cc:440: // Don't send HTTP-Referrer headers.
It's sad, but the header is actually misspelled. The HTTP header is "referer"
not "referrer". Consequentially, I think you should use "referer" here for
consistency.

[jochen (gone - plz use gerrit), 2010-02-09 23:12:43]

On 2010/02/09 23:11:04, ian fette wrote:
> LGTM
> 
> http://codereview.chromium.org/600008/diff/1/2
> File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):
> 
> http://codereview.chromium.org/600008/diff/1/2#newcode465
> chrome/browser/renderer_host/resource_dispatcher_host.cc:465:
> request->set_referrer("");
> std::string() instead of "" as Peter points out below
> 
> http://codereview.chromium.org/600008/diff/1/3
> File chrome/common/chrome_switches.cc (right):
> 
> http://codereview.chromium.org/600008/diff/1/3#newcode440
> chrome/common/chrome_switches.cc:440: // Don't send HTTP-Referrer headers.
> It's sad, but the header is actually misspelled. The HTTP header is "referer"
> not "referrer". Consequentially, I think you should use "referer" here for
> consistency.

I guess you refer to the comment only, not the actual name of the switch?

[Peter Kasting, 2010-02-09 23:14:06]

On 2010/02/09 23:11:04, ian fette wrote:
> http://codereview.chromium.org/600008/diff/1/3#newcode440
> chrome/common/chrome_switches.cc:440: // Don't send HTTP-Referrer headers.
> It's sad, but the header is actually misspelled. The HTTP header is "referer"
> not "referrer". Consequentially, I think you should use "referer" here for
> consistency.

I thought about this but thought it would be better to make the switch spelled
the right way so it was meaningful as a standalone word.

[jochen (gone - plz use gerrit), 2010-02-09 23:15:36]

On 2010/02/09 23:14:06, Peter Kasting wrote:
> On 2010/02/09 23:11:04, ian fette wrote:
> > http://codereview.chromium.org/600008/diff/1/3#newcode440
> > chrome/common/chrome_switches.cc:440: // Don't send HTTP-Referrer headers.
> > It's sad, but the header is actually misspelled. The HTTP header is
"referer"
> > not "referrer". Consequentially, I think you should use "referer" here for
> > consistency.
> 
> I thought about this but thought it would be better to make the switch spelled
> the right way so it was meaningful as a standalone word.

+1 for correct spelling

[jochen (gone - plz use gerrit), 2010-02-09 23:16:38]

http://codereview.chromium.org/600008/diff/1/2
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/600008/diff/1/2#newcode704
chrome/browser/renderer_host/resource_dispatcher_host.cc:704:
request->set_referrer("");
On 2010/02/09 22:39:47, Peter Kasting wrote:
> Nit: "" -> std::string()
> 
> Also you can make this simpler:
> 
> request->set_referrer(CommandLine::ForCurrentProcess()->HasSwitch(
>     switches::kNoReferrer) ? std::string : referrer.spec());

Done.

http://codereview.chromium.org/600008/diff/1/3
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/600008/diff/1/3#newcode440
chrome/common/chrome_switches.cc:440: // Don't send HTTP-Referrer headers.
On 2010/02/09 23:11:04, ian fette wrote:
> It's sad, but the header is actually misspelled. The HTTP header is "referer"
> not "referrer". Consequentially, I think you should use "referer" here for
> consistency.

Done.

http://codereview.chromium.org/600008/diff/1/3#newcode441
chrome/common/chrome_switches.cc:441: const char kNoReferrer[]                  
 = "no-referrer";
On 2010/02/09 22:39:47, Peter Kasting wrote:
> I think "no-referrers" (and pluralizing the switch name) might be a tiny bit
> clearer.

Done.

[darin (slow to review), 2010-02-09 23:28:26]

Hmm... do we need to worry about the case where the
HttpRequestInfo::extra_headers contains Referer?  I don't think this would be
common, but I wonder if it could happen... perhaps via a plugin?

[ian fette, 2010-02-09 23:34:46]

I would personally prefer it if the switch were also "referer" since that is the
header you are disabling, but I feel like we're bikeshedding here so I will say
I don't care.

At the least though, in the comments where you mention the header, you should
say "referer" as that is the name of the header that's being disabled, and if
I'm looking for code related to that header that's what I'm going to search for.

[jochen (gone - plz use gerrit), 2010-02-10 00:37:11]

On 2010/02/09 23:34:46, ian fette wrote:
> I would personally prefer it if the switch were also "referer" since that is
the
> header you are disabling, but I feel like we're bikeshedding here so I will
say
> I don't care.
> 
> At the least though, in the comments where you mention the header, you should
> say "referer" as that is the name of the header that's being disabled, and if
> I'm looking for code related to that header that's what I'm going to search
for.

the method to set the "referer" is set_referrer(). I'll stick with
--no-referrer.

After talking to darin, I'm stripping the header in the http_network_transaction
from extra headers.

[ian fette, 2010-02-10 00:43:10]

On 2010/02/10 00:37:11, jochen wrote:
> On 2010/02/09 23:34:46, ian fette wrote:
> > I would personally prefer it if the switch were also "referer" since that is
> the
> > header you are disabling, but I feel like we're bikeshedding here so I will
> say
> > I don't care.
> > 
> > At the least though, in the comments where you mention the header, you
should
> > say "referer" as that is the name of the header that's being disabled, and
if
> > I'm looking for code related to that header that's what I'm going to search
> for.
> 
> the method to set the "referer" is set_referrer(). I'll stick with
> --no-referrer.
> 

sounds fine.

> After talking to darin, I'm stripping the header in the
http_network_transaction
> from extra headers.

[darin (slow to review), 2010-02-10 06:38:53]

http://codereview.chromium.org/600008/diff/14/18
File net/http/http_network_transaction.cc (right):

http://codereview.chromium.org/600008/diff/14/18#newcode51
net/http/http_network_transaction.cc:51: static const char* const
kExtraHeadersToBeStripped[] = {
please add a comment here explaining why this set of headers are special.  also,
it would be good to add a TODO comment about possibly stripping others.  e.g.,
the user-agent header.

[jochen (gone - plz use gerrit), 2010-02-10 06:52:45]

http://codereview.chromium.org/600008/diff/14/18
File net/http/http_network_transaction.cc (right):

http://codereview.chromium.org/600008/diff/14/18#newcode51
net/http/http_network_transaction.cc:51: static const char* const
kExtraHeadersToBeStripped[] = {
On 2010/02/10 06:38:53, darin wrote:
> please add a comment here explaining why this set of headers are special. 
also,
> it would be good to add a TODO comment about possibly stripping others.  e.g.,
> the user-agent header.

Done.

[darin (slow to review), 2010-02-10 07:20:41]

LGTM

http://codereview.chromium.org/600008/diff/8002/7003
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/600008/diff/8002/7003#newcode619
chrome/browser/renderer_host/resource_dispatcher_host.cc:619:
request->set_referrer(CommandLine::ForCurrentProcess()->HasSwitch(
i think this duplication of code is unfortunate.  we should ideally have a
function that is used to initialize an URLRequest that can be shared.  this
doesn't have to be part of this CL, but if you were inclined to do such
refactoring that'd be great :)

[eroman, 2010-02-11 07:28:45]

Looping-in Jim,

since this will impact DNS prefetching (which is using the referrer to accrue
prefetch benefit.)

[darin (slow to review), 2010-02-11 08:18:56]

Are you sure it is using the referrer?  We often do not have a referrer.  We
should never use the HTTP referrer for anything other than sending it to the
server since it may not be set in several interesting cases.

-Darin



On Wed, Feb 10, 2010 at 11:28 PM, <eroman@chromium.org> wrote:

> Looping-in Jim,
>
> since this will impact DNS prefetching (which is using the referrer to
> accrue
> prefetch benefit.)
>
>
> http://codereview.chromium.org/600008
>

[jar (doing other things), 2010-02-11 18:11:49]

Dns prefetching does indeed use this field.  The prefetching monitors all
navigations, in part, so that it can calculate benefits.  As a second use,
when a navigation takes place to a host that was *not* anticipated
(prefetched), the prefetcher tries to use the referrer field to learn what
it should prefetch (as a subresource of the referrer) in the future.

If there were an alternate slot that carried this information, separate from
the headers, that would indeed be preferable.  Since you noted there are
some cases where we don't fill in this header, it would be a chance to
express a relationship to the prefetcher that is currently missed.

Is there such an alternate slot?  or.... Should I create such a slot?....
and then... in what class do you suggest landing and maintaining it?

Jim

On Thu, Feb 11, 2010 at 12:18 AM, Darin Fisher <darin@chromium.org> wrote:

> Are you sure it is using the referrer?  We often do not have a referrer.
>  We should never use the HTTP referrer for anything other than sending it to
> the server since it may not be set in several interesting cases.
>
> -Darin
>
>
>
> On Wed, Feb 10, 2010 at 11:28 PM, <eroman@chromium.org> wrote:
>
>> Looping-in Jim,
>>
>> since this will impact DNS prefetching (which is using the referrer to
>> accrue
>> prefetch benefit.)
>>
>>
>> http://codereview.chromium.org/600008
>>
>
>

[darin (slow to review), 2010-02-11 18:34:17]

We'd basically need to plumb it all the way back to WebKit if we wanted the
same information as the referrer but applied uniformly to all requests.
 Mozilla has something like this.  Our first-party-for-cookies is almost
what you want, but we have tried to not use that for anything but
third-party cookie blocking.

-Darin


On Thu, Feb 11, 2010 at 10:11 AM, Jim Roskind <jar@chromium.org> wrote:

> Dns prefetching does indeed use this field.  The prefetching monitors all
> navigations, in part, so that it can calculate benefits.  As a second use,
> when a navigation takes place to a host that was *not* anticipated
> (prefetched), the prefetcher tries to use the referrer field to learn what
> it should prefetch (as a subresource of the referrer) in the future.
>
> If there were an alternate slot that carried this information, separate
> from the headers, that would indeed be preferable.  Since you noted there
> are some cases where we don't fill in this header, it would be a chance to
> express a relationship to the prefetcher that is currently missed.
>
> Is there such an alternate slot?  or.... Should I create such a slot?....
> and then... in what class do you suggest landing and maintaining it?
>
> Jim
>
>
> On Thu, Feb 11, 2010 at 12:18 AM, Darin Fisher <darin@chromium.org> wrote:
>
>> Are you sure it is using the referrer?  We often do not have a referrer.
>>  We should never use the HTTP referrer for anything other than sending it to
>> the server since it may not be set in several interesting cases.
>>
>> -Darin
>>
>>
>>
>> On Wed, Feb 10, 2010 at 11:28 PM, <eroman@chromium.org> wrote:
>>
>>> Looping-in Jim,
>>>
>>> since this will impact DNS prefetching (which is using the referrer to
>>> accrue
>>> prefetch benefit.)
>>>
>>>
>>> http://codereview.chromium.org/600008
>>>
>>
>>
>

[jochen (gone - plz use gerrit), 2010-02-11 18:51:01]

I can imagine that this information would be also useful for other upcoming
filtering stuff, so it might by worthwhile to make it a first_party without the
for_cookies

-j

On 2010/02/11 18:34:17, darin wrote:
> We'd basically need to plumb it all the way back to WebKit if we wanted the
> same information as the referrer but applied uniformly to all requests.
>  Mozilla has something like this.  Our first-party-for-cookies is almost
> what you want, but we have tried to not use that for anything but
> third-party cookie blocking.
> 
> -Darin
> 
> 
> On Thu, Feb 11, 2010 at 10:11 AM, Jim Roskind <mailto:jar@chromium.org> wrote:
> 
> > Dns prefetching does indeed use this field.  The prefetching monitors all
> > navigations, in part, so that it can calculate benefits.  As a second use,
> > when a navigation takes place to a host that was *not* anticipated
> > (prefetched), the prefetcher tries to use the referrer field to learn what
> > it should prefetch (as a subresource of the referrer) in the future.
> >
> > If there were an alternate slot that carried this information, separate
> > from the headers, that would indeed be preferable.  Since you noted there
> > are some cases where we don't fill in this header, it would be a chance to
> > express a relationship to the prefetcher that is currently missed.
> >
> > Is there such an alternate slot?  or.... Should I create such a slot?....
> > and then... in what class do you suggest landing and maintaining it?
> >
> > Jim
> >
> >
> > On Thu, Feb 11, 2010 at 12:18 AM, Darin Fisher <mailto:darin@chromium.org>
wrote:
> >
> >> Are you sure it is using the referrer?  We often do not have a referrer.
> >>  We should never use the HTTP referrer for anything other than sending it
to
> >> the server since it may not be set in several interesting cases.
> >>
> >> -Darin
> >>
> >>
> >>
> >> On Wed, Feb 10, 2010 at 11:28 PM, <mailto:eroman@chromium.org> wrote:
> >>
> >>> Looping-in Jim,
> >>>
> >>> since this will impact DNS prefetching (which is using the referrer to
> >>> accrue
> >>> prefetch benefit.)
> >>>
> >>>
> >>> http://codereview.chromium.org/600008
> >>>
> >>
> >>
> >
>

[darin (slow to review), 2010-02-12 06:35:33]

or distinguish the Referer header from the actual referrer.

-darin



On Thu, Feb 11, 2010 at 10:51 AM, <jochen@chromium.org> wrote:

> I can imagine that this information would be also useful for other upcoming
> filtering stuff, so it might by worthwhile to make it a first_party without
> the
> for_cookies
>
> -j
>
>
> On 2010/02/11 18:34:17, darin wrote:
>
>> We'd basically need to plumb it all the way back to WebKit if we wanted
>> the
>> same information as the referrer but applied uniformly to all requests.
>>  Mozilla has something like this.  Our first-party-for-cookies is almost
>> what you want, but we have tried to not use that for anything but
>> third-party cookie blocking.
>>
>
>  -Darin
>>
>
>
>  On Thu, Feb 11, 2010 at 10:11 AM, Jim Roskind <mailto:jar@chromium.org>
>> wrote:
>>
>
>  > Dns prefetching does indeed use this field.  The prefetching monitors
>> all
>> > navigations, in part, so that it can calculate benefits.  As a second
>> use,
>> > when a navigation takes place to a host that was *not* anticipated
>> > (prefetched), the prefetcher tries to use the referrer field to learn
>> what
>> > it should prefetch (as a subresource of the referrer) in the future.
>> >
>> > If there were an alternate slot that carried this information, separate
>> > from the headers, that would indeed be preferable.  Since you noted
>> there
>> > are some cases where we don't fill in this header, it would be a chance
>> to
>> > express a relationship to the prefetcher that is currently missed.
>> >
>> > Is there such an alternate slot?  or.... Should I create such a
>> slot?....
>> > and then... in what class do you suggest landing and maintaining it?
>> >
>> > Jim
>> >
>> >
>> > On Thu, Feb 11, 2010 at 12:18 AM, Darin Fisher <mailto:
>> darin@chromium.org>
>>
> wrote:
>
>> >
>> >> Are you sure it is using the referrer?  We often do not have a
>> referrer.
>> >>  We should never use the HTTP referrer for anything other than sending
>> it
>>
> to
>
>> >> the server since it may not be set in several interesting cases.
>> >>
>> >> -Darin
>> >>
>> >>
>> >>
>> >> On Wed, Feb 10, 2010 at 11:28 PM, <mailto:eroman@chromium.org> wrote:
>> >>
>> >>> Looping-in Jim,
>> >>>
>> >>> since this will impact DNS prefetching (which is using the referrer to
>> >>> accrue
>> >>> prefetch benefit.)
>> >>>
>> >>>
>> >>> http://codereview.chromium.org/600008
>> >>>
>> >>
>> >>
>> >
>>
>
>
>
>
> http://codereview.chromium.org/600008
>