XMLHttpRequest.setRequestHeader() throws NS_ERROR_FAILURE inappropriately

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: zwol, Assigned: Ms2ger)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

Trunk
mozilla17
dev-doc-needed
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
The W3C spec for XMLHttpRequest (see URL) says that setRequestHeader() is supposed to distinguish two kinds of errors, and doesn't license it to produce any other kind of exception:

# Throws an INVALID_STATE_ERR exception if the state is not OPENED or
# if the send() flag is true.
#
# Throws a SYNTAX_ERR exception if header is not a valid HTTP header 
# field name or if value is not a valid HTTP header field value.

Our implementation uses NS_ERROR_FAILURE for conditions falling under both these heads, and also for some internal, should-never-happen conditions (IsPending() or IsCapabilityEnabled() fails - death to xpcom).  And it can throw NS_ERROR_IN_PROGRESS under conditions that are unclear to me but probably shouldn't be visible to JS.

Also, HttpBaseChannel::SetRequestHeader can throw NS_ERROR_INVALID_ARG for conditions that I think are supposed to be mapped to SYNTAX_ERR in this case, and NS_ERROR_NOT_AVAILABLE for another probably should-never-happen condition (nsHttp::ResolveAtom fails).  And several things can produce NS_ERROR_OUT_OF_MEMORY, but we probably don't need to worry about that.
Component: Networking: HTTP → DOM: Mozilla Extensions
QA Contact: networking.http → general
(Reporter)

Comment 1

7 years ago
Hm, if this belongs in DOM:MozillaExtensions then so do a whole bunch of other XMLHttpRequest bugs.
(Assignee)

Updated

7 years ago
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Updated

7 years ago
Assignee: nobody → Ms2ger
Status: NEW → ASSIGNED
(Assignee)

Comment 2

6 years ago
Created attachment 516677 [details] [diff] [review]
WIP

This needs tests. I think I'll submit them to the official test suite first.
(Assignee)

Updated

6 years ago
Whiteboard: [needs tests]
(Assignee)

Updated

6 years ago
Blocks: 704787
(Assignee)

Updated

5 years ago
Blocks: 726433
(Assignee)

Comment 3

5 years ago
Created attachment 645527 [details] [diff] [review]
Patch v1
Attachment #516677 - Attachment is obsolete: true
Attachment #645527 - Flags: review?(bugs)
(Assignee)

Updated

5 years ago
Whiteboard: [needs tests]
(Assignee)

Updated

5 years ago
Attachment #645527 - Flags: review?(bugs) → review?(jonas)
Attachment #645527 - Flags: review?(jonas) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/8426cc6a7875
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: dev-doc-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla17

Comment 5

5 years ago
Backed out with the mass tree revert to get rid of the OS X M5 orange:
https://hg.mozilla.org/mozilla-central/rev/c801b99d726f
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5b07db1a574
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/e5b07db1a574
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Component: DOM: Mozilla Extensions → DOM
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.