bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

"inspect network requests" concatenates multiple header field instances

RESOLVED WONTFIX

Status

DevTools
Console
P3
normal
RESOLVED WONTFIX
5 years ago
a month ago

People

(Reporter: Julian Reschke, Unassigned)

Tracking

Trunk
Bug Flags:
firefox-backlog -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Given a response containing the header fields:

  WWW-Authenticate: Basic realm=foo
  WWW-Authenticate: Basic realm=bar

The network inspector shows

  WWW-Authenticate:Basic realm=foo Basic realm=bar

(copying and pasting shows that the whitespace between "realm=foo" and "Basic" is actually a CR)

The expected result is that the actually received fields are displayed.
(Reporter)

Comment 1

5 years ago
Note this is not security-sensitive; somebody who's authorized please remove the flag.
Group: core-security
(Reporter)

Comment 2

5 years ago
Link to test case that actually sends multiple header fields, in this case WWW-Authenticate:

http://greenbytes.de/tech/tc/httpauth/#multibasicunknown2mf
(Reporter)

Comment 3

5 years ago
Created attachment 727544 [details] [diff] [review]
draft patch

Here's a patch that yields the "right" result. However, it appears that the right way to fix this would be to fix the result of the headers member of httpactivity.
Attachment #727544 - Flags: review?(mcmanus)
Attachment #727544 - Flags: review?(mcmanus) → review?(mihai.sucan)
Thanks julian!
(Reporter)

Comment 5

5 years ago
(In reply to Julian Reschke from comment #3)
> Created attachment 727544 [details] [diff] [review]
> draft patch
> 
> Here's a patch that yields the "right" result. However, it appears that the
> right way to fix this would be to fix the result of the headers member of
> httpactivity.

Expanding on that: the headers list obtained from the HttpActivity currently has one entry per header field name, not per header field instance.

If a header field was received multiple times (as in my example), the returned value just contains the concatenation of the fields, separated by \n.

I *believe* this is the wrong thing to do in the first place and should be changed to actually put multiple entries for the same header field name into the list. However, I have no idea if and what other parts of the code would break.
Comment on attachment 727544 [details] [diff] [review]
draft patch

Review of attachment 727544 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for the bug report and for the patch!

The patch is fine, except we should probably make this change in the web console server code. See the NetworkMonitor in WebConsoleUtils.jsm.

However, I'd like to better understand this bug. Is this really a bug? See HTTP 1/1 specification (RFC 2616), section 4.2, last paragraph. [1] We get the merged header values from Gecko - it's not the Web Console that does the merging (IIANM).

I'm not yet sure if we want this fix. How is this case handled by other developer tools in other web browsers?

[1] http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
Attachment #727544 - Flags: review?(mihai.sucan)

Updated

5 years ago
Assignee: nobody → julian.reschke
Status: NEW → ASSIGNED
Component: Developer Tools: Inspector → Developer Tools: Console
Version: unspecified → Trunk
(Reporter)

Comment 7

5 years ago
(In reply to Mihai Sucan [:msucan] from comment #6)
> Comment on attachment 727544 [details] [diff] [review]
> draft patch
> 
> Review of attachment 727544 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you for the bug report and for the patch!
> 
> The patch is fine, except we should probably make this change in the web
> console server code. See the NetworkMonitor in WebConsoleUtils.jsm.
> 
> However, I'd like to better understand this bug. Is this really a bug? See
> HTTP 1/1 specification (RFC 2616), section 4.2, last paragraph. [1] We get
> the merged header values from Gecko - it's not the Web Console that does the
> merging (IIANM).

Whoever does the merging does it in a non-compliant way; using CR instead of ",".
 
> I'm not yet sure if we want this fix. How is this case handled by other
> developer tools in other web browsers?

I'm pretty sure it should be fixed; what's current displayed is totally incorrect.

Chrome displays it correctly.
This works as intended, for now. We only show the parsed headers. We can also add the raw headers, as an option. Also see bug 859133.

Victor: this is something we can do at the NetworkEventActor level, and in NetworkMonitor. We have Gecko APIs that allow us to also retrieve the raw headers.

Julian: do you want to work on this? I can explain what needs to be done, on IRC or here.
Depends on: 859133
(Reporter)

Comment 9

5 years ago
(In reply to Mihai Sucan [:msucan] from comment #8)
> This works as intended, for now. We only show the parsed headers. We can
> also add the raw headers, as an option. Also see bug 859133.
> ...

How can it be "intended" to show a misleading and incorrect field value?

Updated

5 years ago
Duplicate of this bug: 867819
Priority: -- → P3
Do you still plan to work on this? If yes, a better use of your time would be if you could fix bug 859133. I can mentor you on how to fix it.

If you no longer plan to work on this bug, we should reset the assignee.

Thank you!
Flags: needinfo?(julian.reschke)
(Reporter)

Updated

4 years ago
Assignee: julian.reschke → nobody
Flags: needinfo?(julian.reschke)

Updated

4 years ago
Status: ASSIGNED → NEW

Updated

4 years ago
Flags: firefox-backlog-

Comment 12

3 years ago
This popup has recently been removed.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX

Updated

a month ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.