Referer: not sent for <script src=>

RESOLVED FIXED in mozilla0.9.8

Status

()

Core
DOM
P2
enhancement
RESOLVED FIXED
17 years ago
16 years ago

People

(Reporter: oseberg, Assigned: jst)

Tracking

({dom1, helpwanted})

Trunk
mozilla0.9.8
dom1, helpwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [HAVE FIX], URL)

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
<html>
<head>
</head>
<body>
<script language=javascript src=script.js></script>
</body>
</html>

Does not send the Referer line when requesting, "script.js".

Terje
Blocks: 61660
triaging...
Assignee: asa → darin
Component: Browser-General → Networking: HTTP
QA Contact: doronr → tever

Comment 2

17 years ago
Necko clients must explicitly set the Referer line before reading from an
HTTP channel.  The Referer is otherwise unknown to Necko.

->layout
Assignee: darin → clayton
Component: Networking: HTTP → Layout
QA Contact: tever → petersen

Comment 3

17 years ago
please wontfix this.

If someone objects please cite an RFC as grounds.
Severity: normal → enhancement
OS: Windows NT → All
Hardware: PC → All
Whiteboard: [DONTFIXME]
(Reporter)

Comment 4

17 years ago
RFC 2068                        HTTP/1.1                    January 1997


14.37 Referer

   The Referer[sic] request-header field allows the client to specify,
   for the server's benefit, the address (URI) of the resource from
   which the Request-URI was obtained (the "referrer", although the
   header field is misspelled.) The Referer request-header allows a
   server to generate lists of back-links to resources for interest,
   logging, optimized caching, etc. It also allows obsolete or mistyped
   links to be traced for maintenance. The Referer field MUST NOT be
   sent if the Request-URI was obtained from a source that does not have
   its own URI, such as input from the user keyboard.

        Referer        = "Referer" ":" ( absoluteURI | relativeURI )

   Example:

        Referer: http://www.w3.org/hypertext/DataSources/Overview.html

   If the field value is a partial URI, it SHOULD be interpreted
   relative to the Request-URI. The URI MUST NOT include a fragment.
I'm guessing the DOM folks are responsible for loading scripts.  Reassigning.
Assignee: clayton → jst
Component: Layout → DOM Level 1
QA Contact: petersen → janc
(Assignee)

Comment 6

17 years ago
IE seems to send a referrer when loading scripts but NS 4.x doesn't so unless
someone can give a good reason as to why this is important I won't spend any
time on fixing this. Marking WONTFIX for now, please reopen if there's a strong
case for fixing this (I didn't see anything in the text from the RFC that says a
client *must* send a referrer every time something is requested) or if someone
wants to take this task off my list and fix it.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → WONTFIX
Reopening. This is important for the same reason as referers on images are 
important -- to prevent resource/cycle theft, where another website hosts just
the content but points to scripts on other hosts.

jst: Feel free to move this to nobody@mozilla.org if you don't want to fix it,
but this is a valid RFE and there is no reason why we would _not_ want to have
this behaviour in an ideal world.
Status: RESOLVED → UNCONFIRMED
Keywords: helpwanted, mozilla1.2
Resolution: WONTFIX → ---
Whiteboard: [DONTFIXME]
(Reporter)

Comment 8

17 years ago
Ian, Thank you.

Terje
Valid enhancement request.  Setting status to New.
Ugh.  setting to new for real.  Sorry for the spam...
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 11

17 years ago
Ok, fair enough and I could even fix this given the time to do so but I'm
reassigning to nobody@mozilla.org for now.
Assignee: jst → nobody
Keywords: dom1

Comment 12

17 years ago
QA contact Update
QA Contact: janc → desale

Comment 13

16 years ago
See also bug 74284, referer not send to inline images.

Comment 14

16 years ago
*** Bug 80390 has been marked as a duplicate of this bug. ***

Comment 15

16 years ago
clarified summary for searchability
Summary: Browser does not send referer when requesting a javascript file. → Referer: not sent for <src> tag

Comment 16

16 years ago
Updating QA contact to Shivakiran Tummala.
QA Contact: desale → stummala

Comment 17

16 years ago
Taking...
Assignee: nobody → nisheeth
Target Milestone: --- → mozilla0.9.3
I even saw some sites blocking NS 6.0 because it did not support referer, can't
find the sites now to check if they still block us... I am sure they would also
insist this bug be fixed.

Comment 19

16 years ago
Get a look at http://thedarkexile.dpn.ch, this site is registered on
www.magelo.com and pulls javascript from here to manage various dynamic content.
magelo web site checks the referer to prevent illegal access from unregistered
sites. Since Mozilla doesn't send referers the site doesn't work.

Comment 20

16 years ago
This works for images, so restricting summary.
Summary: Referer: not sent for <src> tag → Referer: not sent for <script src=>

Comment 21

16 years ago
Missed 0.9.3.
Target Milestone: mozilla0.9.3 → mozilla0.9.4

Comment 22

16 years ago
The attached patch sets the referrer url when loading scripts and, consequently, 
 fixes http://thedarkexile.dpn.ch so that it loads properly in Mozilla.

I still have to run Vidur's script loader tests to make sure I didn't break 
anything.  Will then ask for a review and super-review.

If any of you can spot obvious problems in the attached patch, please let me 
know.

Thanks!
Status: NEW → ASSIGNED

Comment 23

16 years ago
Created attachment 47472 [details] [diff] [review]
First attempt at fix

Comment 24

16 years ago
It would help if some of you could test out this patch and report any problems 
you see.  Thanks!

Comment 25

16 years ago
Nisheeth - This has a mozilla1.2 in the keywords . . . Is this really a stop
ship for 0.9.4. If so, please mark it as nsbranch+. If not, pls move it out to
another milestone.

Comment 26

16 years ago
->0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Moving, we really should get this done with as soon as Nisheeth comes back from
vacation.
Priority: -- → P2
Target Milestone: mozilla0.9.5 → mozilla0.9.6

Comment 28

16 years ago
Btw since http://thedarkexile.dpn.ch moved, you can test the fix at 
http://nightwatchers.dpn.ch or directly at http://www.magelo.com

I really hope this fix will be commited to the main tree for the 0.9.6
Thanks in advance.
Johnny, could you please take care of this? Nisheeth made a patch, but he said
he run into some assertion when testing.
Assignee: nisheeth → jst
Status: ASSIGNED → NEW

Comment 30

16 years ago
*** Bug 110040 has been marked as a duplicate of this bug. ***

Comment 31

16 years ago
Guess we missed 0.9.6 ? :(
I really need this bug fixed asap please, this is far from an enhancement for 
me, i cant migrate my dev environement under linux, and users would like to be 
able to use Mozilla/Netscape on their website.

Best regards.
Target Milestone: mozilla0.9.6 → mozilla0.9.7
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.7 → mozilla0.9.8

Comment 32

16 years ago
Problem is difficult to work around in a Server environment.  App requires URLs
to be rewritten so that they are served through a single interface on the wire. 
The only way I can foresee doing this w/o having the Referer field is to
maintain a server-side state table when a request is made, capture that Referer
field and associate it with any <JavaScript SRC= > references so that it can be
used when the browser makes the follow-up request for the JS file.  Any other
suggestions are welcome, but it would be a much cleaner implementation if the
header was sent by the client.

Comment 33

16 years ago
The assertion Nisheeth refers to is probably this thread safety assertion:

###!!! ASSERTION: nsScriptLoadRequest not thread-safe: 'owningThread ==
NS_CurrentThread()', file nsDebug.cpp, line 528
###!!! Break: at file nsDebug.cpp, line 528

Changing nsScriptLoadRequest to use NS_IMPL_THREADSAFE_ISUPPORTS0 makes it go away.

As advertised, the patch makes http://nightwatchers.dpn.ch work.
However, pages from magelo.com itself (http://www.magelo.com, 
http://www.magelo.com/eq_view_profile.html?num=7598, etc) only load partially (a
background appears, which is more than happens without the patch, but nothing
else shows up).  I'm not sure if this is an issue on our end or magelo's.
(Assignee)

Comment 34

16 years ago
Would it not be cleaner to simply add the referrer and referrer flags as
optinonal arguments to NS_NewStreamLoader() and deal with the referrer there in
stead of in the script loader? That would be a far smaller change I would think.
I'll see if I can hack that up...
(Assignee)

Comment 35

16 years ago
Created attachment 63399 [details] [diff] [review]
Add referrer arguments to NS_NewStreamLoader()

Comment 36

16 years ago
Created attachment 63497 [details]
Test Case

Comment 37

16 years ago
Created attachment 63499 [details]
Test Case - Referer check on <script src="">

This is a simple HTML page including a Javascript located at
http://tetrinet.lfjr.net/referercheck.jsp. The query string should be displayed
in the referer. Please ignore my previous test case it didn't work as expected.


Here is the JSP page :

document.getElementById('insertHere').firstChild.nodeValue='<%=
request.getHeader("Referer") %>';
(Assignee)

Comment 38

16 years ago
Thanks for the testcase, Emmanuel.

Darin, Gagan, would you review this stream loader change please?
Status: NEW → ASSIGNED
Whiteboard: [HAVE FIX]

Comment 39

16 years ago
Comment on attachment 63399 [details] [diff] [review]
Add referrer arguments to NS_NewStreamLoader()

r=gagan
Attachment #63399 - Flags: review+

Comment 40

16 years ago
Comment on attachment 63399 [details] [diff] [review]
Add referrer arguments to NS_NewStreamLoader()

>Index: content/base/src/nsScriptLoader.cpp

>-NS_IMPL_ISUPPORTS0(nsScriptLoadRequest)
>+
>+// The nsScriptLoadRequest is passed as the context to necko, and thus
>+// it needs to be threadsafe. Necko won't do anything with this
>+// context, but it will AddRef and Release it on other threads.
>+
>+NS_IMPL_THREADSAFE_ISUPPORTS0(nsScriptLoadRequest)

are you sure?  this shouldn't be the case.

sr=darin
Attachment #63399 - Flags: superreview+
(Assignee)

Comment 41

16 years ago
Yeah, I saw the assertions, some proxying code was notifying some code about
something which ended up QI'ing the nsScriptLoaderRequest on a thread other than
the main thread. IIRC it only happened in an error case, so it doesn't happen
all the time.

Comment 42

16 years ago
would be really great if you could attach the stack trace to that assertion.  it
really shouldn't be happening.  necko consumers aren't supposed to know about
multithreading.
(Assignee)

Comment 43

16 years ago
Here's a stack for the thread-safety assertions:

nsDebug::Assertion(const char * 0x02169cac, const char * 0x1010e32c, const char
* 0x1010e304, int 528) line 290 + 13 bytes
NS_CheckThreadSafe(void * 0x00482e00, const char * 0x02169cac) line 528 + 34 bytes
nsScriptLoadRequest::AddRef(nsScriptLoadRequest * const 0x045e59b0) line 102 +
55 bytes
nsCOMPtr<nsISupports>::nsCOMPtr<nsISupports>(nsISupports * 0x045e59b0) line 787
nsARequestObserverEvent::nsARequestObserverEvent(nsIRequest * 0x045e8ad4,
nsISupports * 0x045e59b0) line 99 + 37 bytes
nsOnStartRequestEvent::nsOnStartRequestEvent(nsRequestObserverProxy *
0x045ee640, nsIRequest * 0x045e8ad4, nsISupports * 0x045e59b0) line 139 + 23 bytes
nsRequestObserverProxy::OnStartRequest(nsRequestObserverProxy * const
0x045ee640, nsIRequest * 0x045e8ad4, nsISupports * 0x045e59b0) line 252 + 39 bytes
nsStreamListenerProxy::OnStartRequest(nsStreamListenerProxy * const 0x045eb040,
nsIRequest * 0x045e8ad4, nsISupports * 0x045e59b0) line 244
nsFileTransport::Process(nsIProgressEventSink * 0x045eb0f0) line 674 + 96 bytes
nsFileTransport::Run(nsFileTransport * const 0x045e8ad8) line 639
nsThreadPoolRunnable::Run(nsThreadPoolRunnable * const 0x0457e230) line 904 + 12
bytes
nsThread::Main(void * 0x0457e1e0) line 120 + 26 bytes
(Assignee)

Comment 44

16 years ago
Fix checked in.

Darin, please reopen or file new bugs if you think necko shouldn't cause the
above assertion to fire when the nsScriptLoadRequest is addreffed/released by
necko, for now nsScriptLoaderRequest has threadsafe AddRef/Release methods.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago16 years ago
Resolution: --- → FIXED

Comment 45

16 years ago
hrm.. thx for the stack trace.. looks like the nsInputStreamChannel requires the
nsISupports context parameter to support threadsafe addref/release.  it really
doesn't have to be that way.  see bug 118820.
*** Bug 128310 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.