Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes for HTTP header compliance. #32024

Merged
merged 1 commit into from Apr 12, 2024
Merged

Fixes for HTTP header compliance. #32024

merged 1 commit into from Apr 12, 2024

Conversation

philip-lamb
Copy link
Contributor

@philip-lamb philip-lamb commented Apr 9, 2024 •
edited

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #___ (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because ___

@mrobinson
Copy link
Member

It looks like this change is failing unit tests ./mach test-unit.

@philip-lamb
Fixes for HTTP header compliance.
84811aa 
- Fix 400 errors from nginx in response to Servo requests by implementing conformant albeit non-normative removal of whitespace from `Accept` and `Accept-Language` HTTP headers. (To match behaviour of Firefox, Safari, and Chrome) https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2
- Provide `Host` header as REQUIRED by HTTP protocol https://www.rfc-editor.org/rfc/rfc9110#field.host
- Update tests.
@mrobinson mrobinson added the T-linux-wpt-2020 Do a try run of the WPT label Apr 10, 2024
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Apr 10, 2024
@github-actions GitHub Actions
Copy link

🔨 Triggering try run (#8638578462) for Linux WPT

@github-actions GitHub Actions
Copy link

Test results for linux-wpt-layout-2020 from try job (#8638578462):

Flaky unexpected result (22)
  • FAIL [expected PASS] /_mozilla/mozilla/iframe/resize_after_load.html (#13573)
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • PASS [expected FAIL] subtest: Matching font-weight: '501' should prefer '390 410' over '300 350'
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '450 460' over '500 501'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 20deg' should prefer 'oblique 20deg' over 'oblique 30deg 60deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -10deg' should prefer 'oblique -5deg' over 'oblique -1deg 0deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -20deg' should prefer 'oblique -60deg -40deg' over 'oblique -10deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -21deg' should prefer 'oblique -60deg -40deg' over 'oblique -10deg'
  • OK /css/cssom-view/MediaQueryList-addListener-removeListener.html (#24569)
    • FAIL [expected PASS] subtest: listeners are called correct number of times

      assert_equals: expected 1 but got 0

  • OK /css/cssom-view/matchMedia.html (#31428)
    • PASS [expected FAIL] subtest: iframe.matchMedia("(width: 200px)") matches
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.https.sub.html (#30111)
    • FAIL [expected TIMEOUT] subtest: sec-fetch-site - Cross-site, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."

    • FAIL [expected NOTRUN] subtest: sec-fetch-site - Same site, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."

    • FAIL [expected NOTRUN] subtest: sec-fetch-site - Same-Origin -> Cross-Site -> Same-Origin redirect, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."

    • FAIL [expected NOTRUN] subtest: sec-fetch-site - Same-Origin -> Same-Site -> Same-Origin redirect, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."

    • TIMEOUT [expected NOTRUN] subtest: sec-fetch-site - Cross-Site -> Same Origin, no attributes

      Test timed out

  • OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
    • FAIL [expected PASS] subtest: Check execution order from nested timeout

      assert_equals: Expected nested setTimeout to run second expected true but got false

    • FAIL [expected PASS] subtest: Check execution order on load handler

      assert_equals: Expected onload to run first expected false but got true

  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
    • PASS [expected FAIL] subtest: Navigating to a different document with form submission
  • TIMEOUT /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-referrer.window.html (#29081)
    • TIMEOUT [expected FAIL] subtest: origin referrer policy used to create the starting page

      Test timed out

    • NOTRUN [expected TIMEOUT] subtest: no-referrer referrer policy used to create the starting page
  • OK /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
    • FAIL [expected PASS] subtest: aElement.click() before the load event must NOT replace

      assert_equals: expected "http://web-platform.test:8000/common/blank.html?thereplacement" but got "http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/resources/code-injector.html?pipe=sub(none)&code=%0A%20%20%20%20const%20a%20%3D%20document.createElement(%22a%22)%3B%0A%20%20%20%20a.href%20%3D%20%22%2Fcommon%2Fblank.html%3Fthereplacement%22%3B%0A%20%20%20%20document.currentScript.before(a)%3B%0A%20%20%20%20a.click()%3B%0A%20%20"

  • OK /html/browsers/history/the-history-interface/traverse_the_history_4.html (#21383)
    • FAIL [expected PASS] subtest: Multiple history traversals, last would be aborted

      assert_array_equals: Pages opened during history navigation expected property 1 to be 5 but got 3 (expected array [6, 5] got [6, 3])

  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
    • FAIL [expected PASS] subtest: Multiple history traversals, last would be aborted

      assert_array_equals: Pages opened during history navigation expected property 1 to be 5 but got 3 (expected array [6, 5] got [6, 3])

  • TIMEOUT [expected OK] /html/browsers/history/the-location-interface/assign-with-nested-iframe.html
  • OK [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if allow-popups-to-escape-sandbox is used

      assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"

  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if allow-popups-to-escape-sandbox is used

      assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"

  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/sandbox-top-navigation-user-activation-sticky.tentative.sub.window.html
    • TIMEOUT [expected FAIL] subtest: Allow top with user activation + user activation

      Test timed out

  • OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: 0x00 in value (formdata event)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: double quote in name (normal form)
  • OK /html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html (#23205)
    • PASS [expected FAIL] subtest: Check that rel=noopener with target=_self does a normal load
  • TIMEOUT [expected OK] /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/remove-initial-about-blankness.window.html (#28684)
    • TIMEOUT [expected FAIL] subtest: Double-check: without document.open(), Window reuse indeed happens

      Test timed out

  • TIMEOUT /html/webappapis/scripting/events/compile-event-handler-settings-objects.html (#24246)
    • TIMEOUT [expected FAIL] subtest: The entry settings object while executing the compiled callback via Web IDL's invoke must be that of the node document

      Test timed out

  • OK [expected ERROR] /workers/constructors/Worker/Worker-constructor.html (#22991)
Stable unexpected results that are known to be intermittent (9)
  • FAIL [expected PASS] /_mozilla/css/dirty_viewport.html (#13731)
  • PASS [expected TIMEOUT] /css/css-fonts/font-size-adjust-reload.html (#30678)
  • PASS [expected FAIL] /css/css-sizing/dynamic-available-size-iframe.html (#31559)
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.sub.html (#30111)
    • FAIL [expected PASS] subtest: sec-fetch-site - Not sent to non-trustworthy same-origin destination, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."

  • OK [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if allow-popups-to-escape-sandbox is used

      assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"

  • CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
  • TIMEOUT /resource-timing/nested-context-navigations-iframe.html (#24311)
    • TIMEOUT [expected PASS] subtest: Test that iframe navigations are not observable by the parent, even after history navigations by the parent

      Test timed out

    • NOTRUN [expected PASS] subtest: Test that crossorigin iframe navigations are not observable by the parent, even after history navigations by the parent
    • NOTRUN [expected PASS] subtest: Test that cross-site iframe navigations are not observable by the parent, even after history navigations by the parent
  • OK [expected TIMEOUT] /webmessaging/with-ports/018.html (#24485)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:
  • TIMEOUT [expected OK] /webmessaging/without-ports/017.html (#24486)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, about:blank

      Test timed out

@github-actions GitHub Actions
Copy link

✨ Try run (#8638578462) succeeded.

@jdm
Copy link
Member

jdm commented Apr 11, 2024

These changes look reasonable to me. I swear hyper used to always provide the host header, but maybe that changed.

@jdm jdm added this pull request to the merge queue Apr 11, 2024
Merged via the queue into servo:main with commit 10ec856 Apr 12, 2024
37 checks passed
@philip-lamb
Copy link
Contributor Author

Thanks Josh.

@philip-lamb philip-lamb deleted the phil-http-header-compliance branch April 12, 2024 03:42
philip-lamb added a commit to philip-lamb/servo that referenced this pull request Apr 16, 2024
@philip-lamb
Backout Host-header related change from servo#32024.
46fd7f1 
servo#32024 broke a different set of nginx sites, and a more sophisticated fix for the case of a missing HTTP 1.1 `Host` header is required.
@philip-lamb philip-lamb mentioned this pull request Apr 16, 2024
Merged
3 tasks
@philip-lamb
Copy link
Contributor Author

Unfortunately, this change breaks a different set of nginx sites, so Host-header-related changes have been backed out in #32093.

github-merge-queue bot pushed a commit that referenced this pull request Apr 16, 2024
@philip-lamb
Backout Host-header related change from #32024. (#32093)
7a507a4 
#32024 broke a different set of nginx sites, and a more sophisticated fix for the case of a missing HTTP 1.1 `Host` header is required.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm jdm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@philip-lamb @mrobinson @jdm