Add more tests for Process-Switching POST loads
Categories
(Core :: DOM: Content Processes, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: nika, Assigned: CuveeHsu)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 2 obsolete files)
The logic added in bug 1467223 should be more thoroughly tested including situations such as those mentioned in bug 1522649, bug 1522640 and bug 1522641.
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Updated•6 years ago
|
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Unassigning from Kashav this requires as lot more context about different ways in which loads can occur, and how to test different layers of necko.
Nhi, could you please assign someone from Necko team on this?
Assignee | ||
Comment 2•5 years ago
|
||
(In reply to Neha Kochar [:neha] from comment #1)
Unassigning from Kashav this requires as lot more context about different ways in which loads can occur, and how to test different layers of necko.
Nhi, could you please assign someone from Necko team on this?
I can take the tests for Necko part, but it's not crystal clear here.
Nika, I have some questions for you, some of them might be out of topic:
(a) Looking at Bug 1522640, I'm wondering what non-Http(s) protocols you want to test.
Fetch doesn't allow the location header to be one url with non-Http(s) scheme.
https://fetch.spec.whatwg.org/#ref-for-concept-response-location-url%E2%91%A3
Or we want to test some error-handling?
(b)
Out of curiosity, why we care most about testing POST?
Looking at bug 1467223, it's subtle between the implementation and enabling POST.
Please skip this question if it's too tedious to answer.
(c)
Besides, I can cook tests like:
(i) a chain of redirections with different origins (origin A to origin B to origin C)
(ii) fetch API with POST, cors or no-cors. (expected in different process for a cross-origin redirect in fetch?)
What else is on top of your head?
Reporter | ||
Comment 3•5 years ago
|
||
(In reply to Junior [:junior] from comment #2)
(a) Looking at Bug 1522640, I'm wondering what non-Http(s) protocols you want to test.
Fetch doesn't allow the location header to be one url with non-Http(s) scheme.
https://fetch.spec.whatwg.org/#ref-for-concept-response-location-url%E2%91%A3Or we want to test some error-handling?
I believe we do occasionally redirect to other types of URLs. For example, it is possible for a webextension to inject an artificial redirect IIRC, which could lead to the destination scheme being a data:
URI.
Both us and chrome also definitely support redirecting from http URI responses to FTP URIs.
As an example, if you clone down WPT to use its server & run something like:
$ mkdir _nikatest
$ vi _nikatest/redirect.py
$ ./wpt serve --doc_root _nikatest
with redirect-handler.py
containing:
def main(request, response):
response.add_required_headers = False
response.writer.write_status(302)
# Did a google search for a ftp server and found this random public-looking one *shrug*
response.writer.write_header("Location", "ftp://speedtest.tele2.net/")
response.writer.end_headers()
response.writer.write("")
Then you'll be able to see the load redirect to the given FTP server upon loading http://web-platform.test:8000/redirect-handler.py
(b)
Out of curiosity, why we care most about testing POST?
Looking at bug 1467223, it's subtle between the implementation and enabling POST.
Please skip this question if it's too tedious to answer.
The reason is just because it was the easiest way to see the situation. there's nothing specific about post other than that the request has to happen from the originating process and not the final process of the process switch due to the request POST data. It also makes it more clear that we can't make the same request twice, because POST loads don't take kindly to that.
(c)
Besides, I can cook tests like:
(i) a chain of redirections with different origins (origin A to origin B to origin C)
(ii) fetch API with POST, cors or no-cors. (expected in different process for a cross-origin redirect in fetch?)
We only expect document loads to change processes, fetch loads will not.
What else is on top of your head?
I worry about dynamically added redirects from extensions, loads of non-http URIs, serviceworker-intercepted loads with synthesized responses, cached & non-cached loads, cached & non-cached redirects, etc.
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
Thanks for those information, Nika. It helps a lot.
I have some minor questions here.
As an example, if you clone down WPT to use its server & run something like:
$ mkdir _nikatest $ vi _nikatest/redirect.py
s/redirect/redirect-handler
:D
$ ./wpt serve --doc_root _nikatest
with `redirect-handler.py` containing:
def main(request, response):
response.add_required_headers = False
response.writer.write_status(302)
# Did a google search for a ftp server and found this random public-looking one shrug
We have good SEO :p
response.writer.write_header("Location", "ftp://speedtest.tele2.net/") response.writer.end_headers() response.writer.write("")
Then you'll be able to see the load redirect to the given FTP server upon loading `http://web-platform.test:8000/redirect-handler.py`
I'm surprised we allow non-Http(s) scheme in location header.
I definitely can do a test for this. And expect the test will be removed once we follow the spec :)
(b)
Out of curiosity, why we care most about testing POST?
Looking at bug 1467223, it's subtle between the implementation and enabling POST.
Please skip this question if it's too tedious to answer.The reason is just because it was the easiest way to see the situation. there's nothing specific about post other than that the request has to happen from the originating process and not the final process of the process switch due to the request POST data. It also makes it more clear that we can't make the same request twice, because POST loads don't take kindly to that.
Got it, thanks.
(c)
Besides, I can cook tests like:
(i) a chain of redirections with different origins (origin A to origin B to origin C)
(ii) fetch API with POST, cors or no-cors. (expected in different process for a cross-origin redirect in fetch?)We only expect document loads to change processes, fetch loads will not.
Cool, looks like we don't need this.
What else is on top of your head?
I worry about dynamically added redirects from extensions
Looks like it's covered by this
loads of non-http URIs,
Does it mean document.location = ftp://FTP_HOST_NAME
?
serviceworker-intercepted loads with synthesized responses
I'm not an expert of sw. Let's see if I can make this test.
Do we change process every time the channel is intercepted?
As I know sw can use cache API
, fetch API
, combination of them or even pure sw generated response.
cached & non-cached loads, cached & non-cached redirects, etc.
Talking about cache and by (b), we're going to test GET here since we don't cache POST.
Assignee | ||
Updated•5 years ago
|
Reporter | ||
Comment 5•5 years ago
|
||
(In reply to Junior [:junior] from comment #4)
Then you'll be able to see the load redirect to the given FTP server upon loading
http://web-platform.test:8000/redirect-handler.py
I'm surprised we allow non-Http(s) scheme in location header.
I definitely can do a test for this. And expect the test will be removed once we follow the spec :)
I should clarify that chrome also supports redirecting to FTP URIs, it acted the same way as us.
(c)
Besides, I can cook tests like:
(i) a chain of redirections with different origins (origin A to origin B to origin C)
(ii) fetch API with POST, cors or no-cors. (expected in different process for a cross-origin redirect in fetch?)We only expect document loads to change processes, fetch loads will not.
Cool, looks like we don't need this.
What else is on top of your head?
I worry about dynamically added redirects from extensions
Looks like it's covered by this
That tests loading extension URIs, not performing synthetic redirects from the webRequest API (e.g. by using the redirectUrl
property on BlockingResponse
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/BlockingResponse#Type)
IIRC you can bypass even more restrictions using that, e.g. I think you can redirect to a data or blob URI (might be wrong on that though).
loads of non-http URIs,
Does it mean
document.location = ftp://FTP_HOST_NAME
?
Yes, that would be an example.
serviceworker-intercepted loads with synthesized responses
I'm not an expert of sw. Let's see if I can make this test.
Do we change process every time the channel is intercepted?
As I know sw can usecache API
,fetch API
, combination of them or even pure sw generated response.
There's another bug around service workers. You can probably leave that test off if you don't understand how it works for now.
cached & non-cached loads, cached & non-cached redirects, etc.
Talking about cache and by (b), we're going to test GET here since we don't cache POST.
That's OK - we need both to work.
Assignee | ||
Comment 6•5 years ago
|
||
(a) Looking at Bug 1522640, I'm wondering what non-Http(s) protocols you want to test.
Fetch doesn't allow the location header to be one url with non-Http(s) scheme.
https://fetch.spec.whatwg.org/#ref-for-concept-response-location-url%E2%91%A3
I'm informed that the restriction only applies for subresource loading.
Sorry for the confusing.
Assignee | ||
Comment 7•5 years ago
|
||
Test file:// and data:
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
Nika, In donig P2, I'm confused with the goal of the test in this bug.
I figure out enabling useHttpResponseProcessSelection
doesn't switch process for different http origins load.
But fission.autostart
does switch.
(Not sure why file:// and http:// switch processes for useHttpResponseProcessSelection
)
Could you hint me the relationship between the two prefs?
And what pref should be enabled under the tests in this bug?
Another finding is:
Under fission.autostart = true
, do page load A->B->C and it comes out three processes instead of procA->procB->procA
Reporter | ||
Comment 10•5 years ago
|
||
(In reply to Junior [:junior] from comment #9)
Nika, In donig P2, I'm confused with the goal of the test in this bug.
I figure out enablinguseHttpResponseProcessSelection
doesn't switch process for different http origins load.
Butfission.autostart
does switch.
Yes, without fission enabled, all http://
URIs load in the same process, even with useHttpResponseProcessSelection
.
(Not sure why file:// and http:// switch processes for
useHttpResponseProcessSelection
)
file URIs load in the file
process, while http URIs load in the web
process, so loading from one to the other would cause a process switch. This is the primary thing which changes when using useHttpResponseProcessSelection
without fission, as the old-style process switches don't support form submission & process switches simultaneously.
Could you hint me the relationship between the two prefs?
And what pref should be enabled under the tests in this bug?Another finding is:
Underfission.autostart = true
, do page load A->B->C and it comes out three processes instead of procA->procB->procA
Is C same-origin with A? If it isn't, then that is intentional. With fission enabled, each origin gets its own process.
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
Use COOP
to force the process switch in redirect with different origins.
Also tests cache each reqeusts.
Updated•5 years ago
|
Assignee | ||
Comment 12•5 years ago
|
||
Assignee | ||
Comment 13•5 years ago
|
||
(In reply to :Nika Layzell (ni? for response) from comment #5)
(In reply to Junior [:junior] from comment #4)
Looks like it's covered by this
That tests loading extension URIs, not performing synthetic redirects from the webRequest API (e.g. by using the
redirectUrl
property onBlockingResponse
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/BlockingResponse#Type)IIRC you can bypass even more restrictions using that, e.g. I think you can redirect to a data or blob URI (might be wrong on that though).
4 months late reply (facepalm)
0:37.34 INFO Console message: [JavaScript Error: "Security Error: Content at moz-extension://UUID/redirect.html may not load or link to file:///PATH/toolkit/components/remotebrowserutils/tests/browser/dummy_page.html."]
Looks like we block loading for some protocol. I'll post the patch later.
Assignee | ||
Comment 14•5 years ago
|
||
Assignee | ||
Comment 15•5 years ago
|
||
data:text/html,%3Ch1%3EHello%2C World!%3C%2Fh1%3E
is good
ftp://speedtest.tele2.net/
looks good (trigger FATAL ERROR: Non-local network connections are disabled and a connection attempt to speedtest.tele2.net (90.130.70.73) was made.)
blob://test
is not good
Let's test with data://
Assignee | ||
Comment 16•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Comment 17•5 years ago
|
||
Comment 20•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3674fdef6f51
https://hg.mozilla.org/mozilla-central/rev/b13610595238
https://hg.mozilla.org/mozilla-central/rev/b0b7702d3923
https://hg.mozilla.org/mozilla-central/rev/f885e475cc2c
Comment 22•5 years ago
|
||
FYI one of the tests that was added here has non-deterministic output on failure (at least in Chromium): html/cross-origin-opener-policy/popup-redirect-cache.https.html
See https://crbug.com/1016762, https://test-results.appspot.com/data/layout_results/linux-rel/223087/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html is an example case where the test failed twice with different outputs.
It would be great if the test could produce deterministic output on failure :)
Assignee | ||
Comment 23•5 years ago
|
||
(In reply to smcgruer from comment #22)
FYI one of the tests that was added here has non-deterministic output on failure (at least in Chromium): html/cross-origin-opener-policy/popup-redirect-cache.https.html
See https://crbug.com/1016762, https://test-results.appspot.com/data/layout_results/linux-rel/223087/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html is an example case where the test failed twice with different outputs.
It would be great if the test could produce deterministic output on failure :)
Fair suggestion. We use uuid for convenience, which could be fixed for a gold star.
https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/testing/web-platform/tests/html/cross-origin-opener-policy/popup-redirect-cache.https.html#55-68
Could you file another bug to address this?
Assignee | ||
Comment 24•5 years ago
|
||
(In reply to Junior [:junior] from comment #23)
(In reply to smcgruer from comment #22)
FYI one of the tests that was added here has non-deterministic output on failure (at least in Chromium): html/cross-origin-opener-policy/popup-redirect-cache.https.html
See https://crbug.com/1016762, https://test-results.appspot.com/data/layout_results/linux-rel/223087/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html is an example case where the test failed twice with different outputs.
It would be great if the test could produce deterministic output on failure :)
Fair suggestion. We use uuid for convenience, which could be fixed for a gold star.
https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/testing/web-platform/tests/html/cross-origin-opener-policy/popup-redirect-cache.https.html#55-68Could you file another bug to address this?
Filed 1592477
Description
•