Last Comment Bug 673296 - Origin handling in EventSource is bogus
: Origin handling in EventSource is bogus
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla10
Assigned To: Wellington Fernando de Macedo
:
Mentors:
Depends on:
Blocks: 664179
  Show dependency treegraph
 
Reported: 2011-07-21 16:44 PDT by Wellington Fernando de Macedo
Modified: 2011-12-11 11:42 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (2.68 KB, patch)
2011-07-21 17:21 PDT, Wellington Fernando de Macedo
bugs: review-
Details | Diff | Splinter Review
v2 (2.31 KB, patch)
2011-07-31 08:44 PDT, Wellington Fernando de Macedo
bugs: review+
jonas: review+
Details | Diff | Splinter Review
v3 (2.29 KB, patch)
2011-10-28 19:06 PDT, Wellington Fernando de Macedo
jonas: review+
Details | Diff | Splinter Review

Description Wellington Fernando de Macedo 2011-07-21 16:44:09 PDT
Similar to bug 670687. See https://bugzilla.mozilla.org/show_bug.cgi?id=664179#c61
Comment 1 Wellington Fernando de Macedo 2011-07-21 17:21:45 PDT
Created attachment 547568 [details] [diff] [review]
v1
Comment 2 Boris Zbarsky [:bz] 2011-07-21 19:46:13 PDT
Why is that getting the origin from the URI and not from the principal?
Comment 3 Olli Pettay [:smaug] 2011-07-22 05:48:08 PDT
Comment on attachment 547568 [details] [diff] [review]
v1

r=me if you address bz' comment.
Comment 4 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-07-22 10:23:08 PDT
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?
Comment 5 Olli Pettay [:smaug] 2011-07-23 06:22:06 PDT
Sicking, did you look at bug 67068?
Comment 6 Olli Pettay [:smaug] 2011-07-23 06:48:48 PDT
Comment on attachment 547568 [details] [diff] [review]
v1

Actually, no, this is not right
Message events don't need ascii representation of the origin.
Comment 7 Olli Pettay [:smaug] 2011-07-23 06:49:35 PDT
"the origin attribute must be set to the Unicode serialization of the origin of the event stream's URL"
Comment 8 Olli Pettay [:smaug] 2011-07-23 06:51:41 PDT
But we may need to do something to the origin,
see https://bugzilla.mozilla.org/show_bug.cgi?id=670687#c5
Comment 9 Olli Pettay [:smaug] 2011-07-23 07:02:03 PDT
nsContentUtils::GetUTFOrigin should do the right thing.


(GetXXXOrigin really should not be in nsContentUtils)
Comment 10 Wellington Fernando de Macedo 2011-07-23 07:46:41 PDT
> 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
Comment 11 Wellington Fernando de Macedo 2011-07-23 07:52:15 PDT
> mUTF16Origin is a somewhat misleading name.
So I will let mOrigin
Comment 12 Wellington Fernando de Macedo 2011-07-31 08:44:41 PDT
Created attachment 549658 [details] [diff] [review]
v2
Comment 13 Olli Pettay [:smaug] 2011-08-01 07:20:43 PDT
Comment on attachment 549658 [details] [diff] [review]
v2

Could we get some tests for this.
Comment 14 Wellington Fernando de Macedo 2011-08-01 14:20:17 PDT
> Could we get some tests for this.
In bug 664179 we have already some tests.
Comment 15 Wellington Fernando de Macedo 2011-10-28 19:06:28 PDT
Created attachment 570446 [details] [diff] [review]
v3

updated for bug 664179
Comment 16 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-10-28 19:08:37 PDT
Comment on attachment 570446 [details] [diff] [review]
v3

You don't actually need to re-request review when just updating to tip.
Comment 17 Wellington Fernando de Macedo 2011-10-28 19:09:50 PDT
> You don't actually need to re-request review when just updating to tip.
Ok, sorry. I didn't know that.
Comment 18 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-10-28 19:59:25 PDT
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.
Comment 19 Marco Bonardo [::mak] 2011-10-29 01:52:31 PDT
https://hg.mozilla.org/mozilla-central/rev/3ab7b95bfcb5
Comment 20 Phil Ringnalda (:philor) 2011-10-29 10:00:51 PDT
Backed out in https://hg.mozilla.org/mozilla-central/rev/abdbf0646a21 - see bug 664179 comment 85
Comment 21 Wellington Fernando de Macedo 2011-12-11 11:42:43 PST
Fixed on bug 664179. See https://bugzilla.mozilla.org/show_bug.cgi?id=664179#c97

Note You need to log in before you can comment on or make changes to this bug.