1511209 - Reftests should not assume that security.fileuri.strict_origin_policy is disabled (except maybe on Windows reftest runs where we need to disable it for perf reasons)

Status: NEW

(Testing :: Reftest, enhancement)

Description

[Daniel Holbert [:dholbert]]

Right now, we run reftests with the about:config pref security.fileuri.strict_origin_policy set to "false", which is Pretty Terrible.

That pref-setting turns off file:// path security-checks, which is what normally prevents HTML files in your ~/Downloads directory from reading /etc/passwd and stuff.

The only reason we disable this pref (I think) is so that we can put reftests into categorized folders, while still letting them share common resources like Ahem.ttf which are stored up a few levels.  But unfortunately this means we can't actually *test* that this security feature is behaving properly.  At least, we can't do so without targeted reenablings of the pref, and we can't really live-tweak the pref anymore as of bug 1286798.

So: I propose we take all of these shared resources (there aren't really that many) and automatically make them available via a local web server, and adjust all of the reftests that use them with relative URIs (e.g ../filters.svg#empty) to reference them using http://[...whatever...]/filters.svg instead.

(This should trivially work for images, but for other things like SVG external-resources and fonts, I think we might need to set some CORS headers or something.)

(I imagine this will share some code with our existing reftest-harness infrastructure from bug 376489 which lets us tag specific reftests as HTTP to have the whole test served over HTTP.)

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> The only reason we disable this pref (I think) is so that we can put
> reftests into categorized folders, while still letting them share common
> resources like Ahem.ttf which are stored up a few levels.

Aha, it's a bit more subtle than this.  Just reconstructing/remembering the history & current state of things.

The *main* reason we turned off this security check was because the security check causes a slowdown on Windows -- so, the pref-disabling was added in bug 846890.

And I'd forgotten until now, but our references to higher-level directories (e.g. "../Ahem.ttf") are Just Fine, regardless of this pref's value, as long as the test in question is itself marked as HTTP(..) (with whatever path it needs) in the test manifest.  So we don't really need to serve resources from a special web server -- we just need to be sure that the tests that *need* those resources are annotated appropriately (and maybe/probably most of them are).  (I had suggested removing those annotations via bug 849081, but now maybe we shouldn't do that.)

So, my current possible-way-forward: maybe we should only turn off this security feature on Windows, since that's where it specifically causes slowness, but reenable the security feature everywhere else. And: for cases where we want to test the security feature, we can just mark those tests as skip-if(windows).

(This is still kinda terrible, but it seems to me like a reasonable tradeoff & a reasonable evolution from where we are at the moment.)

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #1)
> So, my current possible-way-forward: maybe we should only turn off this
> security feature on Windows, since that's where it specifically causes
> slowness, but reenable the security feature everywhere else. And: for cases
> where we want to test the security feature, we can just mark those tests as
> skip-if(windows).

...and we need to add HTTP(..) [or whatever] annotations to all tests that use resources from parent directories.

Sadly it looks like most such tests do not have any annotation like that right now.  I pushed a try run with just the pref-flip, and got massive orange:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=4fa37c7722d878614a59f0fb12709797a22433c0

And here are some commands I ran to find out how many reftests reference parent dirs without bothering with a HTTP annotation:

$ grep -r "\.\.\/" * | cut -f1 -d':' | grep "html\$" | uniq > /tmp/reftests-referencing-parent-dir.txt
$ for x in `cat /tmp/reftests-referencing-parent-dir.txt`; do \
  dir=`echo $x | sed "s%\(.*\)/.*%\1%"`; \
  file=`echo $x | sed "s%.*/\(.*\)%\1%"`; \
  grep -H $file $dir/reftest.list 2>/dev/null; done | grep HTTP | wc -l 

24

$ for x in `cat /tmp/reftests-referencing-parent-dir.txt`; do \
  dir=`echo $x | sed "s%\(.*\)/.*%\1%"`; \
  file=`echo $x | sed "s%.*/\(.*\)%\1%"`; \
  grep -H $file $dir/reftest.list 2>/dev/null; done | grep -v HTTP | wc -l 

817

So, we have 24 files that reference the parent directory and that have an HTTP annotation, and 817 that do not have an annotation (but would need an annotation if we untoggle security.fileuri.strict_origin_policy on some platforms.)