RESOLVED FIXED 164389
Add test infrastructure and tests for existing HTTP 0.9 sandbox machinery
https://bugs.webkit.org/show_bug.cgi?id=164389
Summary Add test infrastructure and tests for existing HTTP 0.9 sandbox machinery
Daniel Bates
Reported 2016-11-03 16:36:01 PDT
As a first step towards improving the HTTP 0.9 security machinery add tests that exercise the current behavior.
Attachments
Patch and layout tests (35.41 KB, patch)
2016-11-03 17:25 PDT, Daniel Bates
no flags
Patch and layout tests (35.64 KB, patch)
2016-11-04 16:19 PDT, Daniel Bates
no flags
Patch and layout tests (34.23 KB, patch)
2016-11-04 20:49 PDT, Daniel Bates
achristensen: review+
Radar WebKit Bug Importer
Comment 1 2016-11-03 16:36:26 PDT
Daniel Bates
Comment 2 2016-11-03 17:25:48 PDT
Created attachment 293834 [details] Patch and layout tests
Brent Fulgham
Comment 3 2016-11-03 20:44:23 PDT
Comment on attachment 293834 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=293834&action=review r=me. > LayoutTests/http/tests/security/http-0.9/default-port-plugin-blocked-expected.txt:3 > +CONSOLE MESSAGE: Sandboxing 'http://127.0.0.1:8000/security/http-0.9/resources/nph-load-plugin-fail.pl' because it is using HTTP/0.9. It's interesting that we hit this three times. Is that expected? > LayoutTests/http/tests/security/http-0.9/default-port-script-blocked-expected.txt:3 > +CONSOLE MESSAGE: Sandboxing 'http://127.0.0.1:8000/security/http-0.9/resources/nph-alert-fail.pl' because it is using HTTP/0.9. Ditto.
Daniel Bates
Comment 4 2016-11-04 09:42:13 PDT
(In reply to comment #3) > > LayoutTests/http/tests/security/http-0.9/default-port-plugin-blocked-expected.txt:3 > > +CONSOLE MESSAGE: Sandboxing 'http://127.0.0.1:8000/security/http-0.9/resources/nph-load-plugin-fail.pl' because it is using HTTP/0.9. > > It's interesting that we hit this three times. Is that expected? > This output reflects the current behavior. The expected behavior is that we emit exactly one console warning. For completeness, the following is a breakdown of the call chain that emits three sandbox messages when we receive an HTTP 0.9 response for a main resource load initiated by an HTTP request to port 80/443 ("a default port"). First SubresourceLoader::didReceiveResponse() is called, which emits one console message by [1]. SubresourceLoader::didReceiveResponse() calls DocumentLoader::responseReceived() at [2] and DocumentLoader::responseReceived() emits one console message by [3]. SubresourceLoader::didReceiveResponse() also calls its base class function, ResourceLoader::didReceiveResponse(), which emits one console message by [4]. Therefore three console messages are emitted. [1] <https://trac.webkit.org/browser/trunk/Source/WebCore/loader/SubresourceLoader.cpp?rev=208112#L267> [2] <https://trac.webkit.org/browser/trunk/Source/WebCore/loader/SubresourceLoader.cpp?rev=208112#L299> [3] <https://trac.webkit.org/browser/trunk/Source/WebCore/loader/DocumentLoader.cpp?rev=207769#L746> [4] <https://trac.webkit.org/browser/trunk/Source/WebCore/loader/ResourceLoader.cpp?rev=208112#L460> > > LayoutTests/http/tests/security/http-0.9/default-port-script-blocked-expected.txt:3 > > +CONSOLE MESSAGE: Sandboxing 'http://127.0.0.1:8000/security/http-0.9/resources/nph-alert-fail.pl' because it is using HTTP/0.9. > > Ditto. See my reply above.
Alex Christensen
Comment 5 2016-11-04 15:21:24 PDT
Comment on attachment 293834 [details] Patch and layout tests Don't touch the URL parser for this
Daniel Bates
Comment 6 2016-11-04 16:19:28 PDT
Created attachment 293954 [details] Patch and layout tests
Daniel Bates
Comment 7 2016-11-04 17:24:14 PDT
Comment on attachment 293954 [details] Patch and layout tests Will update patch following <https://trac.webkit.org/changeset/208407>.
Daniel Bates
Comment 8 2016-11-04 20:49:28 PDT
Created attachment 293981 [details] Patch and layout tests
Alex Christensen
Comment 9 2016-11-07 09:37:01 PST
Comment on attachment 293981 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=293981&action=review > Source/WebCore/platform/URL.cpp:813 > + ASSERT(port > 0); This should be removed and you should use HashMap::find below
Daniel Bates
Comment 10 2016-11-09 13:20:57 PST
Note You need to log in before you can comment on or make changes to this bug.