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

Status: RESOLVED FIXED

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

Description

[Gregory Fleischer]

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

Comment 1

[Gregory Fleischer]

Attachment: 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.

Comment 2

[Josh Matthews [:jdm]]

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.

Comment 3

[Eric Wong (:ekw)]

Attachment: patch

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.

Comment 4

[Josh Matthews [:jdm]]

Thanks Eric! Note that we don't use PRUint32 any more, only the standard types like uint32_t.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 657638 [details] [diff] [review]
patch

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

Can't you use .Contains?

Comment 6

[Eric Wong (:ekw)]

Attachment: patch

Yes, .Contains() works - I didn't know about it.  Patch updated.

Comment 7

[Josh Matthews [:jdm]]

Sorry this was left hanging! This can be checked in.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ryan VanderMeulen from comment #8)
> https://tbpl.mozilla.org/?tree=Try&rev=ba1c84c4a83b

Green on Try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d270033ecaf

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3d270033ecaf