Closed
Bug 1269531
Opened 8 years ago
Closed 7 years ago
Make the Geolocation test suite secure-context aware
Categories
(Core :: DOM: Geolocation, defect)
Core
DOM: Geolocation
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: mds, Assigned: mds)
References
Details
(Whiteboard: btpp-active)
Attachments
(1 file, 1 obsolete file)
Geolocation requests (watch/get) on non-secure origins should be protected by a pref, at least for now.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → michelangelo
Updated•8 years ago
|
Whiteboard: btpp-active
Assignee | ||
Comment 1•8 years ago
|
||
Hey Josh, I have a working patch locally (pretty trivial, though); the real issue is with the mochitests. As almost all the dom/tests/mochi/geolocation tests fail when requiring https, I asked on #Developers how to deal with https-only mochitests. froydnj was suggesting that a good approach may be to have every test "cloned" in order to run the secure (and real) test in a secure context, from a non-secure regular test. For example: 'a.html' can be cloned into 'a-secure.html'. 'a.html' can then call 'a-secure.html' via an HTTPS URL. This - I think - should work but I wonder whether there's a less intricated way to have HTTPS-only test suite.
Flags: needinfo?(josh)
Comment 2•8 years ago
|
||
I do not know of a less invasive way of doing that, unfortunately. The web-platform-tests harness supports adding .https to the filename to run the test from a secure origin, but the mochitest harness does not.
Flags: needinfo?(josh)
Comment 3•8 years ago
|
||
right now we test geolocation via mochitests using a .sjs file: https://dxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/geolocation/network_geolocation.sjs this is referenced via http. I know we support https calls via mochitests using ssltunnel, I am not sure if we support https + .sjs files, it appears we do: https://dxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser_bug553455.js#178 do we have a need to test with non https? If so, then I would agree testing in two modes, or better yet, having each test run in http and https mode. We set the geo server via preferences: https://dxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/geolocation/geolocation_common.js this server could be set in either mode. There is no advantage that I know of to have duplicated files vs a test which does two modes. let me know if I added more confusion and I can clarify anything that might be confusing.
Comment 4•8 years ago
|
||
It's not the sjs that's the problem; it's the HTML test files that are served from an insecure origin (http://mochi.test:888, iirc).
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Josh Matthews [:jdm] from comment #4) > It's not the sjs that's the problem; it's the HTML test files that are > served from an insecure origin (http://mochi.test:888, iirc). That's exactly correct. It'd be great to have some sort of pref somewhere in the mochi suite to shape the way the network layer behaves. Thank you all for all these amazing info, though.
Comment 6•8 years ago
|
||
so change the call from http://mochi.test -> https://mochi.test, am I missing something?
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #6) > so change the call from http://mochi.test -> https://mochi.test, am I > missing something? That is exactly the point.:) Take the tests in dom/tests/mochitest/geolocation; test_allowWatch.html for example. Within that test, we reference navigator.geolocation.watchPosition(...) which is the method that we _MUST_ call on HTTPS for the test to succeed. Whoever calls test_allowWatch.html should do that on HTTPS. This is the blocked I've found myself in. In short, the question is: can we run the dom/tests/mochitest/geolocation suite on HTTPS by default?
Comment 8•8 years ago
|
||
good question and now I see the problem. right now we build a list of tests up and the mochitest harness runs them all via http://, that doesn't help. An easy work around here is to make a wrapper and load each test in an iframe- that iframe could load the test (i.e. test_allowWatch.html) via https://. Would that solve the needs here? In that case you could make test_secure_allowWatch.html that just is load the test via https:// in the iframe- of course you would need to set the geolocation prefs to be https as well.
Assignee | ||
Comment 9•8 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #8) > In that case you could make test_secure_allowWatch.html that just is load > the test via https:// in the iframe- of course you would need to set the > geolocation prefs to be https as well. I really, REALLY, hoped there was some undocumented flip-switch somewhere.:) Thank you, though...:)
Comment 10•8 years ago
|
||
well, we could run ALL tests via https, that would be the super easy hack- not sure if that is what we want to do...although worth considering
Assignee | ||
Comment 11•8 years ago
|
||
For now the pref has been defaulted to true (no change from current behavior). It'll be flipped to false (disallow all non-secure geo requests) as part of the patch for #1072859. Review commit: https://reviewboard.mozilla.org/r/57424/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/57424/
Assignee | ||
Comment 12•8 years ago
|
||
As running all the dom/geolocation mochitests under HTTPS by default doesn't seem a trivial thing to do, I've decided to just commit this patch with the actual code for the preference. Also, the pref is defaulted to 'true', meaning that it still DOES allow non-secure requests. At least I don't get blocked trying to deal with the mochitests.:)
Comment 13•8 years ago
|
||
We should fix up the test harness so that we can run http and/or https. There will be more apis that are only exposed when loaded over secure protocols, thus we'll want the ability to control the context that the test runs in. Joel, can you provide feedback as to exactly how you want this implemented?
Flags: needinfo?(jmaher)
Comment 14•8 years ago
|
||
it appears that just running everything under https://mochi.test:8888/... is not going to work :( Given that data point, I would argue that we need to load a specific test in an iframe or new window as https and it can do the magic work. To do that I assume we would just create a http page that then launches in a new window or iframe the https page which will do the test.
Flags: needinfo?(jmaher)
Assignee | ||
Updated•8 years ago
|
Attachment #8759441 -
Flags: review-
Assignee | ||
Comment 15•8 years ago
|
||
Comment on attachment 8759441 [details] Bug 1269531 - Adding pref for https-only geo reqs. https://reviewboard.mozilla.org/r/57424/#review54892 We need to enable the test harnes to be configurable for secure origins before landing this (see #1278370).
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Summary: Pref switch for non-secure geo requests → Make the Geolocation test suite secure-context aware
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8759441 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8830961 [details] Bug 1269531 - Make the Geolocation test suite secure-context aware. https://reviewboard.mozilla.org/r/107622/#review108774 Nice and straightfoward.
Attachment #8830961 -
Flags: review?(josh) → review+
Assignee | ||
Comment 19•7 years ago
|
||
(In reply to Josh Matthews [:jdm] from comment #18) > Nice and straightfoward. Thanks, I appreciate it!:)
Comment 20•7 years ago
|
||
Pushed by mdesimone@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b747db3970bd Make the Geolocation test suite secure-context aware. r=jdm
Comment 21•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b747db3970bd
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•