756021 - resolved provider origin is incomplete

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

In manifestDB.jsm, the function   function normalizeOrigin(aURL) {

Returns an incomplete origin.  Given the URL 'http://mochi.test:8888/' the origin is resolved as 'mochi.test' where it should include the protocol and port (ie, it should be 'http://mochi.test:8888/'.

While the functionality matches the comments in that function, https://developer.mozilla.org/en/Same_origin_policy_for_JavaScript indicates it probably is wrong.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Note that once this is fixed, head.js in the m-c branch tests will need to be updated accordingly else many tests will fail.

Comment 2

[Shane Caraveo (:mixedpuppy)]

origin may not be the correct name.  this is used as the db key to ensure that only one provider per domain is installable.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Just to un-clarify, from IRC:

(10:02:10 AM) markh: I thought [apps] were port proto-host-port, not just host
(10:02:34 AM) michaelhanson: they are - but I need to talk with the security team about that one
(10:02:56 AM) michaelhanson: we had a conversation yesterday about whether http://site.com could perhaps tickle an API for "please turn on social browsing for me" when the provider was registered for https://site.com
(10:03:49 AM) markh: michaelhanson: sorry, "they are" which? :)
(10:03:58 AM) michaelhanson: oh, apps are per origin
(10:04:07 AM) markh: right - so we should be too?
(10:04:19 AM) michaelhanson: subject to the caveat I just posted there, yah
(10:04:34 AM) michaelhanson: I don't want to get into a world where there's a secure- and a non-secure version of the same site installed

So - in theory it should be per-origin rather than per-host, but http vs https is a tricky edge case...

Comment 4

[Mark Hammond [:markh] [:mhammond]]

FWIW, bug 756317 tracks the fact that webapps basically does do what we do but should *not* strip the port.  So it seems we should end up *without* the protocol but with any specified port.

Comment 5

[Shane Caraveo (:mixedpuppy)]

pushed a fix for host+port origin as the db key

https://github.com/mozilla/socialapi-dev/commit/a1f3652372281e93413a6f05df2e1b6e05891631

Comment 6

[Mark Hammond [:markh] [:mhammond]]

The tests are testing this, even if a little indirectly.  There is an explicit test that currentProvider.origin == TEST_PROVIDER_ORIGIN where TEST_PROVIDER_ORIGIN = "http://mochi.test:8888";

Comment 7

[Adam Muntner [:adamm] (use NEEDINFO)]

Can someone explain how much of nsIURI is being used? I don't have a clear sense from the convo on this bug.

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIURI

The conversation leads me to a few q's:

1. Since we are using the scheme (http/https) shouldn't only HTTPS be allowed, since only HTTPS is used for providers, and they should be using CSP?

So we might have:

someplace.com:443
someplace.com:8888

with https being implicit?

As such, there should never be an oppty for a non-secure site being set up, since a manifest should never load from a http-only url, with the cert passing.

---

2. It looks like the bug above is a patch to here

https://github.com/mozilla/socialapi-dev/blob/e668ec63bb4e942fae6bbb36c2261c9f8caa76c6/modules/registry.js

i am pretty new to using github, but it looks like it's not merged into the main develop branch /modules path

https://github.com/mozilla/socialapi-dev/tree/develop/modules

Is there a place you guys are tracking what's merged/not merged and the plan for that? You guys seem to have a good handle on it, but following the app, it's not always straightforward for me to see something as resolved/fixed when the code is in some kind of unmerged branch and/or what this seems to be, an alternate tree. It might just be my misunderstaing of github, so I could prob def benefit from a brief explanation or url reference.