If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

web console reports data which the server did not send (304 Not Modified)

VERIFIED FIXED in Firefox 4.0b10

Status

()

Firefox
Developer Tools
P2
normal
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: Stefan, Assigned: msucan)

Tracking

Trunk
Firefox 4.0b10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [patchclean:0107][hardblocker], URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20100101 Firefox/4.0b7pre
Build Identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20100101 Firefox/4.0b7pre

In the 304 (Not Modified) case the web console reports "Response Headers" which the server did not sent in its response to the request.

Reproducible: Always

Steps to Reproduce:
1. load http://www.heise.de/imgs/02/5/7/4/6/7/9/7b8c4e77d197c9bf.jpeg
2. open web console
3. press reload
4. click at the entry in the log
Actual Results:  
web console reports "Response Headers":

Accept-Ranges:bytes
Content-Length:7608
Content-Type:image/jpeg
Date:Sun, 26 Sep 2010 13:23:06 GMT
Last-Modified:Sat, 25 Sep 2010 16:48:31 GMT
Server:Apache

Expected Results:  
Report only these headers which the server actually sent:

<HTTP/1.1 304 Not Modified>
<Date: Sun, 26 Sep 2010 13:23:06 GMT>
<Server: Apache>
<Connection: close>
(Reporter)

Updated

7 years ago
Summary: web console reports data which where not sent by the server (304 Not Modified) → web console reports data which the server did not send (304 Not Modified)

Updated

7 years ago
Component: General → Developer Tools
QA Contact: general → developer.tools
Version: unspecified → Trunk
I'm getting a 404 response for that URL now.

In any case, you're seeing the original headers from the request stored along with the cached response body. We need to keep those headers around to know when to expire the cached item. You can also so those headers in about:cache.

not a bug.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → INVALID
(In reply to comment #1)
> ... You can also so* those headers in about:cache.

*see
(Reporter)

Comment 3

7 years ago
Take this image which is still available:

http://www.heise.de/icons/ho/heise_online_logo.gif

My proxy reports that only these response headers are sent over the network:

<HTTP/1.1 304 Not Modified>
<Date: Mon, 08 Nov 2010 14:41:20 GMT>
<Server: Apache>
<Connection: close>

while the web console says (let me quote the window title: "Inspect Network Request"):

Accept-Ranges:bytes
Content-Length:3416
Content-Type:image/gif
Date:Mon, 08 Nov 2010 14:41:20 GMT
Last-Modified:Sat, 26 Dec 2009 14:00:00 GMT
Server:Apache

Since the program does not deliver what it says it's self-evidently a bug. It delivers in part cached information and sells them under the label "fresh from the network". 

Aside from that the bug renders the use of the web console useless when debugginng 304 related errors since one can't tell which information is from the actual server response.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---

Comment 4

7 years ago
I agree. At the very least, it should indicate that the response headers are from the cache, but it would be more useful to show the actual headers sent from the server.
Blocks: 593956
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

7 years ago
Assignee: nobody → mihai.sucan

Comment 5

7 years ago
mass change: filter on PRIORITYSETTING
Priority: -- → P2
(Assignee)

Comment 6

7 years ago
Problem investigated:

- Firebug 1.5.4 in Firefox 3.6 has the same bug.

- There are no obvious issues with the Web Console code. We get the headers from the nsIHttpChannel/nsIRequest. We don't pick and choose - it's all raw there. (or it is supposed to be).

- The Live HTTP Headers 0.16 extension in Firefox 3.6 shows the response headers correctly. Inspecting the source code shows no obvious technical differences. It takes the nsIHttpChannel and uses the available visitResponseHeaders() method, just like the Web Console does.

There's only one nice catch: we use the nsIHttpActivityDistributor, while the Live HTTP Headers extension uses the nsIObserver service, to listen for the http-on-examine-response topic. It gets the subject (the nsIHttpChannel) which gives access to the headers, and so on.

We set an nsIStreamListener (our ResponseListener object) on the nsIHttpChannel (QI to nsITraceableChannel). We check for available response headers in OnDataAvailable() and OnStopRequest().

At the moment, I have a hacked, "debug quality" (ugly code), HUDService which uses the nsIObserver, and it works - it is able to read the real response headers. However, it does not yet tie into the network panel code, because the activity distributor only signals that the request starts *after* the nsIObserver sends notification for http-on-examine-response. (no httpActivity object in openRequests). I believe I can work around this issue, nonetheless.

Do you guys have better ideas than adding yet another observer? I'll prepare a patch that's actually usable, and we can further the discussion then.

(It seems to me like this would be a "Gecko bug", because it never says we get different headers when we use the nsIHttpActivityDistributor.)
(Assignee)

Comment 7

7 years ago
Created attachment 493504 [details] [diff] [review]
proposed patch

Proposed patch. This adds a new observer for http-on-examine-response. This observer stores the HTTP response headers, that can be used by the other network panel code.

Directly storing the headers in openRequests (in httpActivity objects stored there), did not work, because for some reason http-on-examine-response is "almost" simultaneously ran while the other HTTP activity observer runs. Both cannot see changes made to the object from the other observer. I was forced to use a separate object where I can store the response headers, and I was also forced to read them at the end of the request (onStopRequest), otherwise it would have been "too early" (the response headers object would not visible earlier).

The fix is not too ugly, but it's somehow unfortunate we have to use an additional observer.

The mochitest case does pretty much the same as the STR of this bug. It opens a page, which has 200 OK, then reload to get 304 Not Modified (with different headers). I was glad to see that such important network code changes did not break old tests.

Looking forward to your feedback. Thanks!
Attachment #493504 - Flags: feedback?
(Assignee)

Updated

7 years ago
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [patchclean:1127]
(Assignee)

Updated

7 years ago
Attachment #493504 - Flags: feedback? → feedback?(rcampbell)
Comment on attachment 493504 [details] [diff] [review]
proposed patch

+    // This is used to find the correct HTTP response headers.
+    this.httpResponseExaminer = {
+      observe: function (aSubject, aTopic)

You don't need to create a separate observe function within the httpResponseExaminer object. Just make httpResponseExaminer the function and pass it to your addObserver call.

Not super keen on the additional observer, but this looks like it would do the job.

I'm wondering if it's possible to poll the cache in our onClick handler for http response objects and get the real header when we open the network inspector instead?
(Assignee)

Comment 9

7 years ago
(In reply to comment #8)
> Comment on attachment 493504 [details] [diff] [review]
> proposed patch
> 
> +    // This is used to find the correct HTTP response headers.
> +    this.httpResponseExaminer = {
> +      observe: function (aSubject, aTopic)
> 
> You don't need to create a separate observe function within the
> httpResponseExaminer object. Just make httpResponseExaminer the function and
> pass it to your addObserver call.

Ah, yeah. I'll update the patch.

> I'm wondering if it's possible to poll the cache in our onClick handler for
> http response objects and get the real header when we open the network
> inspector instead?

Thought of that as well. If we try to call visitResponseHeaders() too early or too late we get an exception (NOT AVAILABLE). We can only do it in onDataAvailable and onStopRequest. In both places we get cached headers, no the real ones.

I only know of http-on-examine-response, from the Live HTTP Headers extension, and this, based on testing, really gives us the real HTTP headers. If we want a different solution, I think we need to ping someone from the platform. Please CC someone accordingly.

Thanks!
(Assignee)

Comment 10

7 years ago
Created attachment 494402 [details] [diff] [review]
patch update 2

Updated the patch as requested. Thanks for looking into it, Robert!
Attachment #493504 - Attachment is obsolete: true
Attachment #494402 - Flags: feedback?
Attachment #493504 - Flags: feedback?(rcampbell)
(Assignee)

Updated

7 years ago
Attachment #494402 - Flags: feedback? → feedback?(rcampbell)
(Assignee)

Updated

7 years ago
Whiteboard: [patchclean:1127] → [patchclean:1201]
Attachment #494402 - Flags: feedback?(rcampbell) → feedback+
(In reply to comment #9)
> (In reply to comment #8)
> > Comment on attachment 493504 [details] [diff] [review] [details]
> > I'm wondering if it's possible to poll the cache in our onClick handler for
> > http response objects and get the real header when we open the network
> > inspector instead?
> 
> Thought of that as well. If we try to call visitResponseHeaders() too early or
> too late we get an exception (NOT AVAILABLE). We can only do it in
> onDataAvailable and onStopRequest. In both places we get cached headers, no the
> real ones.

Was hoping that would work, but it was a long shot. f+!
(Assignee)

Comment 12

7 years ago
Comment on attachment 494402 [details] [diff] [review]
patch update 2

Thanks for the feedback+! Asking for review.
Attachment #494402 - Flags: review?(dolske)
Blocks: 606055
(Assignee)

Comment 13

7 years ago
For reference: I reported bug 615936 about the nsIHttpActivityObserver not giving us the uncached HTTP response headers, as we were expecting.
Whiteboard: [patchclean:1201] → [patchbitrot]
Doesn't apply; could you update when you get a chance? Sorry I didn't get to this sooner.
(Assignee)

Comment 15

7 years ago
Created attachment 502028 [details] [diff] [review]
patch update 3

Patch rebased.
Attachment #494402 - Attachment is obsolete: true
Attachment #502028 - Flags: review?(dolske)
Attachment #494402 - Flags: review?(dolske)
(Assignee)

Updated

7 years ago
Whiteboard: [patchbitrot] → [patchclean:0107]
blocking2.0: --- → ?
This is the fix for bug 606055, which is a nasty hard blocker.
Attachment #502028 - Flags: feedback+
blocking2.0: ? → final+
Whiteboard: [patchclean:0107] → [patchclean:0107][softblocker]
Changing the whiteboard to hard blocker. We really need a review on this.
Whiteboard: [patchclean:0107][softblocker] → [patchclean:0107][hardblocker]
Comment on attachment 502028 [details] [diff] [review]
patch update 3

Looks fine, but I'm mostly leaning on pwalton's review to have caught any nitpicky things.
Attachment #502028 - Flags: review?(dolske) → review+
Keywords: checkin-needed

Comment 19

7 years ago
http://hg.mozilla.org/mozilla-central/rev/635f0075c184
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b10
Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10) Gecko/20100101 Firefox/4.0b10 ID:20110121161358
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.