Last Comment Bug 628966 - test_bug428847.html (by way of file_bug428847-1.xhtml) loads http://www.mozilla.com/whatever.xsl over the network
: test_bug428847.html (by way of file_bug428847-1.xhtml) loads http://www.mozil...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla5
Assigned To: Phil Ringnalda (:philor)
:
Mentors:
Depends on:
Blocks: 630089 428847 626999
  Show dependency treegraph
 
Reported: 2011-01-26 05:43 PST by Ted Mielczarek [:ted.mielczarek]
Modified: 2011-03-30 20:37 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.1-fixed
.17-fixed
.19-fixed


Attachments
Trunk and 2.1 and 2.0 - Use example.com and a todo (1.35 KB, patch)
2011-01-30 17:41 PST, Phil Ringnalda (:philor)
jonas: review+
Details | Diff | Review
1.9.2 and 1.9.1 - just example.com (513 bytes, patch)
2011-02-13 18:11 PST, Phil Ringnalda (:philor)
no flags Details | Diff | Review
1.9.1 - example.com and proxying (1.48 KB, patch)
2011-02-13 18:13 PST, Phil Ringnalda (:philor)
no flags Details | Diff | Review

Description Ted Mielczarek [:ted.mielczarek] 2011-01-26 05:43:12 PST
I ran mochitest-1/5 with Wireshark running and caught this test making a HTTP request:

GET /whatever.xsl HTTP/1.1
Host: www.mozilla.com
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20110105 Firefox/4.0b9pre
Accept: text/xml,application/xml,application/xhtml+xml,*/*;q=0.1
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip, deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 115
Connection: keep-alive
Referer: http://mochi.test:8888/tests/content/base/test/file_bug428847-1.xhtml
Origin: http://mochi.test:8888

We should fix this to load the stylesheet from the test server (using a mochi.test or example.com URL).
Comment 1 Phil Ringnalda (:philor) 2011-01-30 16:49:21 PST
I'm puzzled about what happened here.

Apparently the point of the test was that we would deny the load of http://www.mozilla.com/whatever.xsl (so it didn't matter that it wasn't local, because we weren't going to load it), and the test checked whether we still loaded the page that tried to apply it, rather than displaying an error message.

Then, going by http://hg.mozilla.org/mozilla-central/rev/f1af6bb87895, apparently cross-site XHR made it so that we did load it, and then... I guess applying that 404 page as a stylesheet resulted in something which didn't have a <body>.

If we *meant* to allow any page to load any other cross-origin page as a stylesheet, then the test should be removed, rather than just having the part where we test the results commented out.

If there are some sorts of cross-origin stylesheet loads that are still denied (file:/// maybe?), then the test should be trying to load one of them instead.

If we totally didn't mean to break denying cross-origin stylesheet loads, then rather than commenting out the test telling us we did, we should be blocking on fixing that.
Comment 2 Phil Ringnalda (:philor) 2011-01-30 16:50:33 PST
Curse you, Bugzilla, I wanted sicking getting https://bugzilla.mozilla.org/show_bug.cgi?id=628966#c1 in his bugspam, not getting a second comment waving up the page saying "read that, please?"
Comment 3 Phil Ringnalda (:philor) 2011-01-30 17:33:38 PST
Oh, I guess I could just run the blessed test and see how it's failing, couldn't I? I take back my nom here, I can't morph this enough to be both the bug about hitting the network and the bug about breaking the test.

What's happening is that we're getting a "" error message, so cross-site XHR regressed bug 428847, but the right fix *here* is to change the PI to load something local (and to change from commenting out a failing test to making it a todo). The regression, and whether it should block, needs to be a separate bug.
Comment 4 Phil Ringnalda (:philor) 2011-01-30 17:36:55 PST
Copy-paste is hard. A "Error loading stylesheet: A network error occurred loading an XSLT stylesheet: http://www.mozilla.com/whatever.xsl" error message.
Comment 5 Phil Ringnalda (:philor) 2011-01-30 17:41:47 PST
Created attachment 508304 [details] [diff] [review]
Trunk and 2.1 and 2.0 - Use example.com and a todo

Fixes the network access by using the proxied example.com, and switches the commented-out test to be a todo.
Comment 6 Phil Ringnalda (:philor) 2011-02-13 18:11:34 PST
Created attachment 512080 [details] [diff] [review]
1.9.2 and 1.9.1 - just example.com

Since the test isn't broken on 1.9.2
Comment 7 Phil Ringnalda (:philor) 2011-02-13 18:13:34 PST
Created attachment 512082 [details] [diff] [review]
1.9.1 - example.com and proxying

And it isn't broken on 1.9.1, but www.example.com is because when I was proxying it on 1.9.2 I didn't yet need it on 1.9.1
Comment 8 Phil Ringnalda (:philor) 2011-02-13 19:06:09 PST
Comment on attachment 512082 [details] [diff] [review]
1.9.1 - example.com and proxying

Wound up needing the www.example.com proxying for another patch, so this one won't need to land it.
Comment 9 Ben Hearsum (:bhearsum) 2011-03-23 10:32:51 PDT
Johnny, any chance you can review this soon? We'd really like to turn off outbound access for build machine soon, and this is blocking it.
Comment 11 Ted Mielczarek [:ted.mielczarek] 2011-03-24 05:37:04 PDT
Needs a 2.1 landing for sanity, probably.
Comment 12 :Ehsan Akhgari (busy, don't ask for review please) 2011-03-24 12:26:42 PDT
http://hg.mozilla.org/mozilla-central/rev/ff5717c3e48c
Comment 13 :Ehsan Akhgari (busy, don't ask for review please) 2011-03-24 12:29:15 PDT
http://hg.mozilla.org/releases/mozilla-2.1/rev/4c176899228b

If somebody suggests that I should land this on 1.9 too, they won't see my smile.  Just sayin'.  ;-)
Comment 14 Phil Ringnalda (:philor) 2011-03-24 19:40:47 PDT
If anything runs tests on 1.9 and shows its head by hitting the network, that's just a handy way to spot something that's gone rogue and needs to be shot. Thanks again for the huge stream of checkins :)
Comment 15 Ben Hearsum (:bhearsum) 2011-03-25 10:06:02 PDT
Thanks everyone!

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