Closed Bug 64248 Opened 24 years ago Closed 23 years ago

Referer: not sent for <script src=>

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

Product:
Core
Component:
DOM: Core & HTML
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla0.9.8

People

(Reporter: oseberg, Assigned: jst)

References

(
URL
)

Details

(Keywords: dom1, helpwanted, Whiteboard: [HAVE FIX])

Attachments

(4 files)

First attempt at fix
7.78 KB, patch
Details | Diff | Splinter Review
Add referrer arguments to NS_NewStreamLoader()
5.78 KB, patch
gagan
: review+
darin.moz
: superreview+
Details | Diff | Splinter Review
Test Case
585 bytes, text/html
Details
Test Case - Referer check on <script src="">
226 bytes, text/html
Details 
<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
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
please wontfix this.

If someone objects please cite an RFC as grounds.
Severity: normal → enhancement
OS: Windows NT → All
Hardware: PC → All
Whiteboard: [DONTFIXME]
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
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
Closed: 24 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]
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
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
QA contact Update
QA Contact: janc → desale
See also bug 74284, referer not send to inline images.
*** Bug 80390 has been marked as a duplicate of this bug. ***
clarified summary for searchability
Summary: Browser does not send referer when requesting a javascript file. → Referer: not sent for <src> tag
Updating QA contact to Shivakiran Tummala.
QA Contact: desale → stummala
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.
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.
This works for images, so restricting summary.
Summary: Referer: not sent for <src> tag → Referer: not sent for <script src=>
Missed 0.9.3.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
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
It would help if some of you could test out this patch and report any problems 
you see.  Thanks!
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.
->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
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
*** Bug 110040 has been marked as a duplicate of this bug. ***
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
Target Milestone: mozilla0.9.7 → mozilla0.9.8
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.
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.
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...
Attached file Test Case — 

    
    

    
  


  
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") %>';

    
    

    
  
Thanks for the testcase, Emmanuel.

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

    
    

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

r=gagan

        Attachment #63399 -
        Flags: review+

    
    

    
  
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+

    
    

    
  
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.

    
    

    
  
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.

    
    

    
  
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


    
    

    
  
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
Closed: 24 years ago → 23 years ago
Resolution: --- → FIXED

    
    

    
  
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. ***
Component: DOM → DOM: Core & HTML

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: