For HTTP/2 statusText (both Fetch and XMLHttpRequest) should always be the empty byte sequence

NEW
Unassigned

Status

()

P3
normal
2 years ago
11 days ago

People

(Reporter: annevk, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

2 years ago
See https://github.com/whatwg/fetch/issues/599.

I don't have tests unfortunately as web-platform-tests doesn't support HTTP/2.
If we need to get it prioritized, please let us know.
Priority: -- → P3
(Reporter)

Comment 2

a year ago
Moving this to Networking as it seems more likely this is an issue with our underlying implementation.
Component: DOM → Networking
(Reporter)

Updated

a year ago
Summary: For HTTP/2 responseText (both Fetch and XMLHttpRequest) should always be the empty byte sequence → For HTTP/2 statusText (both Fetch and XMLHttpRequest) should always be the empty byte sequence

Comment 3

a year ago
This is almost certainly because our h2 implementation is effectively an h1 translation layer, so there's very little outside necko that cares if a request is h2. To help with potential assumptions about a status phrase existing, when we translate responses from h2 into h1, we synthesize status phrases (see bug 950910 - also used for h1, so not something we want to get rid of entirely).

We should evaluate whether we want to globally do away with status phrases for h2 (which I'm hesitant to do - being able to read the status phrases (in, say, dev tools) makes it easier for people to understand what happened with their request; could also have other fallout we're not anticipating) or if we just want to force an empty statusText in the DOM for fetch & xhr over h2.

If we go with the latter (what I'm inclined towards), this would be more appropriate living in DOM. There, you can see if the connection is h2 (you should be able to use nsIHttpChannel.GetProtocolVersion to figure that out) and if so, just ignore any status phrase the http layer reports.
Component: Networking → DOM
(Reporter)

Comment 4

a year ago
Nicholas, I think bug 950910 should be reverted to some extent. From a protocol standpoint they don't exist for HTTP/2, so requiring each endpoint to mask them to avoid exposing them to web content seems rather risky. If tooling needs this, why can't we add them there?

In HTTP/1 it's also optional and can basically be any string, so making something up (even if the default suggested phrase) seems somewhat incorrect.
Flags: needinfo?(hurley)

Comment 5

11 months ago
(In reply to Anne (:annevk) from comment #4)
> Nicholas, I think bug 950910 should be reverted to some extent. From a
> protocol standpoint they don't exist for HTTP/2, so requiring each endpoint
> to mask them to avoid exposing them to web content seems rather risky. If
> tooling needs this, why can't we add them there?
> 
> In HTTP/1 it's also optional and can basically be any string, so making
> something up (even if the default suggested phrase) seems somewhat incorrect.

I still disagree. As you mention, for http/1 at least, the status phrase can be absolutely anything. In fact, for the better part of 20 years, if we didn't receive a status phrase, necko would just put one that says "OK" in place, regardless of the code. We made it nicer a few years back. In fact, we even synthesize status phrases for http/0.9, which doesn't even have a status *line*, let alone a phrase!

So that leaves us with a few issues. (1) gecko has 20 years of expecting status phrases everywhere, and the chances of breaking someone expecting a status phrase that I haven't found is non-trivial (this goes double for the fact that http status lines are available to extensions, who may be depending on us providing a status phrase as we always have). (2) content-level consumers of the status phrase really shouldn't care what it is (since, as you note, it can be just about anything in h1, and there's no guarantee that any particular connection will be h2). (3) It remains the case that having explanatory status text available is very useful. To be honest, I'm not even sure the fetch or xhr specs should be enforcing *anything* about statusText, given its history and flexible nature.

Given all that, if the DOM *really* wants to insist on statusText being the empty string for h2 connections, I think the DOM should enforce that. I don't see it being worth the technical risk of changing necko just to satisfy an attempt to be strict on something that has historically not been particularly strict.
Flags: needinfo?(hurley)
Flags: needinfo?(bzbarsky)
Flags: needinfo?(bzbarsky)
(Reporter)

Comment 6

11 months ago
To be clear, it's not about insisting, it's about what's being standardized and being interoperable with other browsers. And this includes extensions, which share APIs and code with equivalent extensions for Chrome, Edge, and Safari. So by exposing something here we have the risk that code not anticipating that will end up failing in Firefox. And by not fixing it Networking we end up with an architecture that is a mismatch with standards and future expectations of any new API around this.
Flags: needinfo?(hurley)

Comment 7

11 months ago
And I'm arguing that perhaps we shouldn't be trying to standardize something that has no particularly standard format to begin with, nor can you a priori say that a particular resource will be coming over h2, so you can't just expect "hey, this connection will be h2, and therefore I shouldn't see a statusText". Yes, you can tell after the fact if something came over h2, but we *already* make changes to h2 responses (injecting headers, for example), so adding a status phrase isn't, like, totally out of the realm of sanity.

Either way, I can tell that this conversation is likely to continue in circles. As such, I'll ni?mcmanus for a module owner's decision.
Flags: needinfo?(hurley) → needinfo?(mcmanus)
have you just considered taking status text out of the fetch specification? What does it add? If it adds something that isn't version specific, why wouldn't our approach be right?

I actually think we're doing the right thing - it has a semantic but no literal value in h2. (in h1 it has both). We're reporting the semantic.

any change here would need to consider the impact to devtools (which is no doubt solvable).
Flags: needinfo?(mcmanus)
(Reporter)

Comment 9

11 months ago
If I had known (or someone had told me) it would be slowly removed from HTTP at the time we created Response I'd never have added it, though not sure if it matters much as exposed in XMLHttpRequest too. I suspect that removing it from both places is likely not web compatible and if we can't remove it from both it's not really worth the trouble removing it from either.

I just had a look at devtools and I only found it reporting the status code, coupled with a link to MDN to learn more about the status code. Where does it report the status text?
Flags: needinfo?(mcmanus)
looks like its just under raw headers in devtools now.. I think the more info link is 'new' and welcome. There might be other places that consume it too..
Flags: needinfo?(mcmanus)
(Reporter)

Comment 11

11 months ago
I'm confused. On the one hand the argument seems to be that this field is meaningless, but on the other hand there's a concern over breaking internal consumers. (It appears the concern over devtools was not valid.) If there's already concern over breaking internal consumers, why isn't it much bigger concern for external consumers, and doesn't that mean we should address this in a way that minimizes risk and aligns with other browsers?

Comment 12

11 months ago
External consumers already have to handle just about anything (empty string, meaningful text, a meaningless sequence of (printable?) bytes) in statusText. Internal consumers may, in fact, depend on us having something meaningful to display (or at the very least, that the status phrase isn't the empty string). This field, as pointed out multiple times previously, is in no way particularly "standard" anywhere (except on the wire in h2). I think there needs to be *very* strong evidence that standardizing this field will be a *large* benefit to external consumers before we should expend the standardization effort.
(Reporter)

Comment 13

11 months ago
The effort is already expended as far as the APIs under consideration are concerned... This bug was filed as a result of that effort.

Anne, is a decision here something we should prioritize? Based on the discussion, I'm not sure if it would be worthwhile to start with just having XHRs and Fetch Responses return a blank statusText if their channel.GetProtocolVersion == "h2"?

Flags: needinfo?(annevk)
(Reporter)

Comment 15

11 days ago

I think that's worthwhile doing. If we can do it at a lower-level that would be better, but not making it web-exposed seems like a win as it might become some kind of interoperability issue.

I suspect HTTP/3 and whatever comes after will also not expose this, so we might want to be forward-compatible with that in the check.

Flags: needinfo?(annevk)
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.