|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
59 bytes, text/x-review-board-request
|Details | Review|
Geolocation requests (watch/get) on non-secure origins should be protected by a pref, at least for now.
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.
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.
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.
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).
(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.
(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?
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.
(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...:)
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
Created attachment 8759441 [details] Bug 1269531 - Adding pref for https-only geo reqs. 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/
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.:)
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?
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.
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).
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.
(In reply to Josh Matthews [:jdm] from comment #18) > Nice and straightfoward. Thanks, I appreciate it!:)
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/b747db3970bd Make the Geolocation test suite secure-context aware. r=jdm