258185 - Current referer sent when pressing shift-enter in location field

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect, P1)

Description

[Martin 'Douglas' Schuster]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040704 Debian/1.4-2 StumbleUpon/1.76
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040704 Debian/1.4-2 StumbleUpon/1.76

Mozilla sends the last URL that was generated by the sidebar as Referer when
entering another URL in the location field and pressing Shift-Enter (for download).

Reproducible: Always
Steps to Reproduce:
1. Open sidebar
2. Use google-search in sidebar, look for "testing"
3. Type http://server.whose.access.log.i.can.read/foobar in the location field
and press shift-enter

Actual Results:  
access.log on the server: Referer will be set to
"http://www.google.com/search?q=testing&sourceid=mozilla&start=0&start=0&ie=utf-8&oe=utf-8"

Expected Results:  
Referer should have been empty

Comment 1

[Martin 'Douglas' Schuster]

I also have seen this bug when using
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko
/20040803
so it's not Linux- or Debian- related, I guess.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

> Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko/20040803

did you install any extensions on that build?

Comment 3

[Martin 'Douglas' Schuster]

No extensions were installed on the Windows-build at that time.

Comment 4

[Daniel Veditz [:dveditz]]

Privacy issue, but not an exploit: clearing security flag.

Has nothing to do with sidebar, it appears to be whatever happens to be in the
content window is sent when you shift-enter (download/save) the location bar
url. It should not send the referrer, just as we don't if you type into the
location bar and load it into the browser window.

Could be embarassing :-)

0[263f18]: http request [
0[263f18]:   HEAD / HTTP/1.1
0[263f18]:   Host: www.mozilla.org
...
0[263f18]:   Referer:
http://www.google.com/search?hl=en&ie=UTF-8&q=bedwetting&btnG=Google+Search
0[263f18]: ]

Comment 5

[Hixie (not reading bugmail)]

This might explain why I get a lot of bogus referrers from Firefox UAs on my
blog, although I don't understand why people would be saving my page...

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

this is entirely in frontend code, fixing component. taking, but I'll wait with
fixing this until bug 160454 gets checked in, which is rewriting the involved code.

see the unchecked usages of getReferrer in contentAreaUtils.js for the involved
code.

this could be called a regression, because for a while we sent no referrer...

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 8

[[e-mail]]

Comment on attachment 163321 [details] [diff] [review]
patch

so, why are you putting aReferrer before aChosenData when we're not actually
using aChosenData yet?

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

neil: hm, no real reason. I can change it if you want (I'd prefer not to add a
new patch just to switch the order of the two params, though)

Comment 10

[[e-mail]]

Comment on attachment 163321 [details] [diff] [review]
patch

>+  var referrer = null;
>+  if (aDocument.referrer)
>+    referrer = makeURI(aDocument.referrer, aDocument.characterSet);
>+
>   if (aDocument)
Need to null check aDocument more here.

I noticed a couple of places where you could have used makeURI.

Comment 11

[[e-mail]]

Comment on attachment 163321 [details] [diff] [review]
patch

Cancelling due to bitrot.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

(for my reference: bug 239472)

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: updated patch

Comment 14

[[e-mail]]

Comment on attachment 181720 [details] [diff] [review]
updated patch

What about the changes to pageInfo.js saveMedia() and contentAreaUtils.js
getReferrer() [note that the old change looks inaccurate] and saveDocument()
[which was also incorrect]?

>+          saveURL(href, linkNode ? gatherTextUnder(linkNode) : "",
>+                  "SaveLinkTitle", false, getReferrer(document));
Sorry that I missed this in the previous review, but "false" is not documented
as a legal value for aChosenData. I told you you should have put aReferrer
first :-P

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #14)
> (From update of attachment 181720 [details] [diff] [review] [edit])
> What about the changes to pageInfo.js saveMedia() and contentAreaUtils.js
> getReferrer() [note that the old change looks inaccurate] and saveDocument()
> [which was also incorrect]?

pageInfo now does pass a referrer (bug 239472)

contentAreaUtils.js - you mean the charset passing? how does it look incorrect?

> Sorry that I missed this in the previous review, but "false" is not documented
> as a legal value for aChosenData. I told you you should have put aReferrer
> first :-P

That's not untrue, but saveURL does not have an aChosenData argument?
http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/contentAreaUtils.js#219

Comment 16

[[e-mail]]

Comment on attachment 181720 [details] [diff] [review]
updated patch

OK, so you lost the getReferrer() charset change, but I don't think that was
correct anyway. Note to self: do reviews during daylight hours!

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 181720 [details] [diff] [review]
updated patch

fixes a privacy problem, only affects seamonkey

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 181720 [details] [diff] [review]
updated patch

a=dveditz

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in xpfe/communicator/resources/content/contentAreaClick.js;
/cvsroot/mozilla/xpfe/communicator/resources/content/contentAreaClick.js,v  <--
 contentAreaClick.js
new revision: 1.44; previous revision: 1.43
done
Checking in xpfe/communicator/resources/content/contentAreaUtils.js;
/cvsroot/mozilla/xpfe/communicator/resources/content/contentAreaUtils.js,v  <--
 contentAreaUtils.js
new revision: 1.129; previous revision: 1.128
done
Checking in xpfe/communicator/resources/content/nsContextMenu.js;
/cvsroot/mozilla/xpfe/communicator/resources/content/nsContextMenu.js,v  <-- 
nsContextMenu.js
new revision: 1.101; previous revision: 1.100
done