Closed Bug 504240 Opened 15 years ago Closed 12 years ago

Duplicated header values in Access-Control-Request-Headers for preflight cross-site XMLHttpRequest requests

Categories

(Core :: DOM: Core & HTML, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: gfleischer+bugzilla, Assigned: ekw)

References

()

Details

(Whiteboard: [mentor=jdm][lang=c++])

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.0.11) Gecko/2009060214 Firefox/3.0.11
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.1pre) Gecko/20090714 Shiretoko/3.5.1pre

If the same custom header is added multiple times in a cross-site XMLHttpRequest request, it is repeated in the Access-Control-Request-Headers header value in the preflight OPTIONS request.

Reproducible: Always
Attached file Testcase
Test case will make cross-site XMLHttpRequest with the same header added multiple times.  The preflight request is stored and then returned in the response.
Component: General → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: general → general
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: student-project
In http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsXMLHttpRequest.cpp, the uses of mCORSUnsafeHeaders.AppendElement should probably be wrapped in a method that checks whether the header already exists in mCORSUnsafeHeaders.
Whiteboard: [mentor=jdm][lang=c++]
Attached patch patch (obsolete) — Splinter Review
I'd like to work on this bug.  Please assign to me.

In this patch, I didn't write a wrapper around mCORSUnsafeHeaders.AppendElement() as suggested in comment 2 because it is only used twice, and only one causes the problem in this bug.  But if warranted on review, I can write the wrapper.
Attachment #657638 - Flags: feedback?(josh)
Assignee: nobody → ewong3
Thanks Eric! Note that we don't use PRUint32 any more, only the standard types like uint32_t.
Attachment #657638 - Flags: feedback?(josh) → review?(jonas)
Comment on attachment 657638 [details] [diff] [review]
patch

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

Can't you use .Contains?
Attached patch patchSplinter Review
Yes, .Contains() works - I didn't know about it.  Patch updated.
Attachment #657638 - Attachment is obsolete: true
Attachment #657638 - Flags: review?(jonas)
Attachment #658788 - Flags: review?(jonas)
Sorry this was left hanging! This can be checked in.
Pushed to Try since I don't see any links here. I'll land it if it's green.
https://tbpl.mozilla.org/?tree=Try&rev=ba1c84c4a83b
https://hg.mozilla.org/mozilla-central/rev/3d270033ecaf
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.