Origin handling in EventSource is bogus

RESOLVED FIXED in mozilla10

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Wellington Fernando de Macedo, Assigned: Wellington Fernando de Macedo)

Tracking

Trunk
mozilla10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Similar to bug 670687. See https://bugzilla.mozilla.org/show_bug.cgi?id=664179#c61
(Assignee)

Updated

6 years ago
Blocks: 664179
(Assignee)

Updated

6 years ago
Assignee: nobody → wfernandom2004
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

6 years ago
Created attachment 547568 [details] [diff] [review]
v1
Attachment #547568 - Flags: review?(jonas)
Attachment #547568 - Flags: review?(Olli.Pettay)

Comment 2

6 years ago
Why is that getting the origin from the URI and not from the principal?
Comment on attachment 547568 [details] [diff] [review]
v1

r=me if you address bz' comment.
Attachment #547568 - Flags: review?(Olli.Pettay) → review+
Comment on attachment 547568 [details] [diff] [review]
v1

Hmm.. mUTF16Origin is a somewhat misleading name. GetASCIIOrigin returns the puny-code encoded origin. I would have expected mUTF16Origin to contain a unicode encoded origin based on the explicit 'utf16' in the name.

I'd also have guessed that the event should contain the unicode variant.

Is there a reason you explicitly ask for the punycode coded variant?
Sicking, did you look at bug 67068?
Comment on attachment 547568 [details] [diff] [review]
v1

Actually, no, this is not right
Message events don't need ascii representation of the origin.
Attachment #547568 - Flags: review+ → review-
"the origin attribute must be set to the Unicode serialization of the origin of the event stream's URL"
But we may need to do something to the origin,
see https://bugzilla.mozilla.org/show_bug.cgi?id=670687#c5
nsContentUtils::GetUTFOrigin should do the right thing.


(GetXXXOrigin really should not be in nsContentUtils)
(Assignee)

Comment 10

6 years ago
> Why is that getting the origin from the URI and not from the principal?
Because "the origin attribute must be set to the Unicode serialization of the origin of the event stream's URL". 

I don't know if I have misunderstood that, but I think per spec it is correct. WebSockets uses the origin from the principal, which is get from the entry script. It seems strange to me, but the specs define these behaviors...

> nsContentUtils::GetUTFOrigin should do the right thing.
ok
(Assignee)

Comment 11

6 years ago
> mUTF16Origin is a somewhat misleading name.
So I will let mOrigin
(Assignee)

Comment 12

6 years ago
Created attachment 549658 [details] [diff] [review]
v2
Attachment #547568 - Attachment is obsolete: true
Attachment #547568 - Flags: review?(jonas)
Attachment #549658 - Flags: review?(jonas)
Attachment #549658 - Flags: review?(Olli.Pettay)
Comment on attachment 549658 [details] [diff] [review]
v2

Could we get some tests for this.
Attachment #549658 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 14

6 years ago
> Could we get some tests for this.
In bug 664179 we have already some tests.
Attachment #549658 - Flags: review?(jonas) → review+
(Assignee)

Comment 15

6 years ago
Created attachment 570446 [details] [diff] [review]
v3

updated for bug 664179
Attachment #549658 - Attachment is obsolete: true
Attachment #570446 - Flags: review?(jonas)
Comment on attachment 570446 [details] [diff] [review]
v3

You don't actually need to re-request review when just updating to tip.
Attachment #570446 - Flags: review?(jonas) → review+
(Assignee)

Comment 17

6 years ago
> You don't actually need to re-request review when just updating to tip.
Ok, sorry. I didn't know that.
Checked in to inbound:

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

Thanks for the quick turnaround and for the patch in general!! Please do poke someone if you see bugs lingering like this, that should never happen.
https://hg.mozilla.org/mozilla-central/rev/3ab7b95bfcb5
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Backed out in https://hg.mozilla.org/mozilla-central/rev/abdbf0646a21 - see bug 664179 comment 85
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 21

6 years ago
Fixed on bug 664179. See https://bugzilla.mozilla.org/show_bug.cgi?id=664179#c97
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.