fix http cache reconstruct response from cache after validation behaviour by rayguo17 · Pull Request #40067 · servo/servo

rayguo17 · 2025-10-22T02:56:15Z

In the scenario of constructing response from cache resources after successful validation of expired cache resouce, Instead of just copying full body of the cached_resource, check if range header exist in request, if so reuse handle_range_request function to construct response.

Testing: Should past the manual test case in Issue #40050
Fixes: #40050

yezhizhen · 2025-10-22T03:55:05Z

This seems to fail some http cache unit test.

github-actions · 2025-10-22T10:13:31Z

🔨 Triggering try run (#18712870431) for Linux (WPT)

github-actions · 2025-10-22T10:41:54Z

Test results for linux-wpt from try job (#18712870431):

Flaky unexpected result (24)

TIMEOUT /FileAPI/url/url-in-tags-revoke.window.html (#19978)
- TIMEOUT [expected PASS] subtest: Fetching a blob URL immediately before revoking it works in <script> tags.
```
Test timed out
```
OK /IndexedDB/idbfactory_open.any.html
- FAIL [expected PASS] subtest: Calling open() with version argument 1.5 should not throw.
```
assert_equals: version expected 1 but got 9007199254740991
```
OK /IndexedDB/idbobjectstore_get.any.worker.html (#38852)
- FAIL [expected PASS] subtest: Attempts to retrieve a record that doesn't exist
```
assert_unreached: unexpected open.success event Reached unreachable code
```
OK /_mozilla/mozilla/getBoundingClientRect.html (#39668)
- FAIL [expected PASS] subtest: getBoundingClientRect 1
```
assert_equals: expected 62 but got 60.35
```
OK /_webgl/conformance/textures/misc/texture-upload-size.html (#21770)
- PASS [expected FAIL] subtest: WebGL test #45
- PASS [expected FAIL] subtest: WebGL test #47
- PASS [expected FAIL] subtest: WebGL test #49
- PASS [expected FAIL] subtest: WebGL test #51
- FAIL [expected PASS] subtest: WebGL test #53
```
assert_true: Texture was smaller than the expected size 2x2 expected true got false
```
- FAIL [expected PASS] subtest: WebGL test #55
```
assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false
```
- FAIL [expected PASS] subtest: WebGL test #57
```
assert_true: Texture was smaller than the expected size 2x2 expected true got false
```
- FAIL [expected PASS] subtest: WebGL test #59
```
assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false
```
- PASS [expected FAIL] subtest: WebGL test #61
- PASS [expected FAIL] subtest: WebGL test #63
- And 6 more unexpected results...
OK /custom-elements/form-associated/ElementInternals-setFormValue.html (#29174)
- PASS [expected FAIL] subtest: Single value - name is missing
OK /fetch/content-length/api-and-duplicate-headers.any.html (#35873)
- FAIL [expected PASS] subtest: fetch() and duplicate Content-Length/Content-Type headers
```
promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
```
OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin-fragment.html (#20768)
- FAIL [expected PASS] subtest: Tests that a fragment navigation in the unload handler will not block the initial navigation
```
assert_equals: expected "" but got "#fragment"
```
OK /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
- PASS [expected FAIL] subtest: aElement.click() before the load event must NOT replace
OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
- PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/document-with-fragment-empty.html (#28259)
- TIMEOUT [expected FAIL] subtest: Autofocus elements in top-level browsing context's documents with empty fragments should work.
```
Test timed out
```
TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/document-with-fragment-nonexistent.html (#28259)
- TIMEOUT [expected FAIL] subtest: Autofocus elements in top-level browsing context's documents with non-existent fragments should work.
```
Test timed out
```
TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/document-with-fragment-top.html (#28259)
- TIMEOUT [expected FAIL] subtest: Autofocus elements in top-level browsing context's documents with "top" fragments should work.
```
Test timed out
```
TIMEOUT /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
- FAIL [expected TIMEOUT] subtest: Element with tabindex should support autofocus
```
assert_equals: expected "SPAN" but got "BODY"
```
- TIMEOUT [expected NOTRUN] subtest: Non-HTMLElement should not support autofocus
```
Test timed out
```
OK /html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/allow-scripts-flag-changing-2.html (#39703)
- FAIL [expected PASS] subtest: Meta refresh of the original iframe is not blocked if moved into a sandboxed iframe
```
uncaught exception: Error: assert_unreached: The iframe into which the meta was moved must not refresh Reached unreachable code
```
OK [expected TIMEOUT] /html/semantics/embedded-content/media-elements/playing-the-media-resource/loop-from-ended.tentative.html (#33778)
- FAIL [expected TIMEOUT] subtest: play() with loop set to true after playback ended
```
this argument is not a finite floating-point value
```
OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
- PASS [expected FAIL] subtest: text/plain: Basic test (normal form)
OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
- FAIL [expected PASS] subtest: application/x-www-form-urlencoded: Basic test (normal form)
```
assert_equals: expected "basic=test" but got ""
```
OK /preload/link-header-preload-delay-onload.html (#39622)
- FAIL [expected PASS] subtest: Makes sure that Link headers preload resources and block window.onload after resource discovery
```
assert_true: expected true got false
```
OK /preload/prefetch-document.html (#37210)
- FAIL [expected PASS] subtest: different-site document prefetch with 'as=document' should not be consumed
```
assert_equals: expected 2 but got 1
```
CRASH [expected OK] /referrer-policy/gen/top.http-rp/strict-origin-when-cross-origin/iframe-tag.http.html
OK /trusted-types/trusted-types-navigation.html?01-05 (#38975)
- PASS [expected FAIL] subtest: Navigate a window via anchor with javascript:-urls in enforcing mode.
- PASS [expected FAIL] subtest: Navigate a frame via anchor with javascript:-urls in enforcing mode.
OK /trusted-types/trusted-types-navigation.html?31-35 (#38034)
- PASS [expected FAIL] subtest: Navigate a frame via form-submission with javascript:-urls in report-only mode.
OK [expected TIMEOUT] /webmessaging/with-ports/017.html (#24486)
- PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, about:blank

Stable unexpected results that are known to be intermittent (21)

OK /IndexedDB/idbcursor-continuePrimaryKey-exceptions.any.worker.html (#39277)

FAIL [expected PASS] subtest: IDBCursor continuePrimaryKey() on object store cursor

assert_throws_dom: continuePrimaryKey() should throw if source is not an index function "function() {
        cursor.continuePrimaryKey(2, 2);
      }" threw object "TypeError: cursor.continuePrimaryKey is not a function" that is not a DOMException InvalidAccessError: property "code" is equal to undefined, expected 15

OK /IndexedDB/idbobjectstore_getAll.any.html (#39276)
- PASS [expected FAIL] subtest: Get all values with transaction.commit()
OK /IndexedDB/idbobjectstore_getAll.any.worker.html (#39400)
- PASS [expected FAIL] subtest: Get all values with transaction.commit()

OK /IndexedDB/key-conversion-exceptions.any.html (#39305)

FAIL [expected PASS] subtest: IDBCursor continue() method with throwing/invalid keys

assert_throws_exactly: key conversion with throwing getter should rethrow function "() =&gt; {
      receiver[method](key);
    }" threw object "TypeError: receiver[method] is not a function" but we expected it to throw object "getter: throwing from getter"

OK /IndexedDB/key-conversion-exceptions.any.worker.html (#39284)

FAIL [expected PASS] subtest: IDBCursor continue() method with throwing/invalid keys

assert_throws_exactly: key conversion with throwing getter should rethrow function "() =&gt; {
      receiver[method](key);
    }" threw object "TypeError: receiver[method] is not a function" but we expected it to throw object "getter: throwing from getter"

FAIL [expected PASS] /_mozilla/mozilla/sslfail.html (#10760)
TIMEOUT [expected OK] /_mozilla/mozilla/window_resize_event.html (#36741)
- TIMEOUT [expected PASS] subtest: Popup onresize event fires after resizeTo
```
Test timed out
```
TIMEOUT [expected OK] /credential-management/credentialscontainer-frame-basics.https.html (#39430)
- TIMEOUT [expected FAIL] subtest: navigator.credentials should be undefined in documents generated from data: URLs.
```
Test timed out
```
TIMEOUT [expected FAIL] /dom/xslt/large-cdata.html (#38029)
OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
- PASS [expected FAIL] subtest: sec-fetch-site - Same origin
- FAIL [expected PASS] subtest: sec-fetch-user
```
promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
```
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Cross-site
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Same site
OK /fetch/metadata/generated/css-font-face.sub.tentative.html (#34624)
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy same-origin destination
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy same-site destination
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy cross-site destination
TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
- TIMEOUT [expected PASS] subtest: background-image sec-fetch-dest - Not sent to non-trustworthy cross-site destination
```
Test timed out
```
ERROR /fetch/metadata/generated/serviceworker.https.sub.html (#36247)
- FAIL [expected PASS] subtest: sec-fetch-site - Same origin, no options - registration
```
promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
```
OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
- PASS [expected FAIL] subtest: Check execution order on load handler
- PASS [expected FAIL] subtest: Check execution order from nested timeout
OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/autofocus-dialog.html (#29087)
TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_other_frame_popup.sub.html (#39702)
- TIMEOUT [expected FAIL] subtest: Sandboxed iframe can not navigate other frame's popup
```
Test timed out
```
OK [expected CRASH] /html/semantics/forms/the-fieldset-element/disabled-003.html (#31730, #39631)
OK /navigation-timing/test-navigation-type-reload.html (#33334)
- FAIL [expected PASS] subtest: Reload domComplete > Original domComplete
```
assert_true: Reload domComplete &gt; Original domComplete expected true got false
```
- FAIL [expected PASS] subtest: Reload domContentLoadedEventEnd > Original domContentLoadedEventEnd
```
assert_true: Reload domContentLoadedEventEnd &gt; Original domContentLoadedEventEnd expected true got false
```
- FAIL [expected PASS] subtest: Reload domContentLoadedEventStart > Original domContentLoadedEventStart
```
assert_true: Reload domContentLoadedEventStart &gt; Original domContentLoadedEventStart expected true got false
```
- FAIL [expected PASS] subtest: Reload fetchStart > Original fetchStart
```
assert_true: Reload fetchStart &gt; Original fetchStart expected true got false
```
- FAIL [expected PASS] subtest: Reload loadEventEnd > Original loadEventEnd
```
assert_true: Reload loadEventEnd &gt; Original loadEventEnd expected true got false
```
- FAIL [expected PASS] subtest: Reload loadEventStart > Original loadEventStart
```
assert_true: Reload loadEventStart &gt; Original loadEventStart expected true got false
```
OK /preload/preload-error.sub.html (#37177)
- PASS [expected FAIL] subtest: CORS (fetch): main
OK /trusted-types/trusted-types-navigation.html?26-30 (#38807)
- FAIL [expected PASS] subtest: Navigate a window via form-submission with javascript:-urls in report-only mode.
```
promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""
```
- PASS [expected FAIL] subtest: Navigate a frame via form-submission with javascript:-urls in enforcing mode.
TIMEOUT [expected OK] /webstorage/localstorage-about-blank-3P-iframe-opens-3P-window.partitioned.html (#29053)
- TIMEOUT [expected PASS] subtest: StorageKey: test 3P about:blank window opened from a 3P iframe
```
Test timed out
```

github-actions · 2025-10-22T10:42:07Z

✨ Try run (#18712870431) succeeded.

TimvdLippe

Can you add a new WPT test following the steps in https://book.servo.org/hacking/testing.html ? You can add the test in https://github.com/servo/servo/tree/main/tests/wpt/tests/fetch/range and most likely you can use non-matching-range-response.html as inspiration.

jdm · 2025-10-22T19:29:40Z

Implementation-wise this looks reasonable. I agree that I'd like to see an automated test covering this situation!

…iour Signed-off-by: rayguo17 <[email protected]>

servo-wpt-sync · 2025-10-23T12:02:06Z

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#55613) with upstreamable changes.

TimvdLippe

Lots of stylistic nits. The main concern is that the test as written doesn't pass in Firefox or Safari (yet it passes in Chrome based on its logs). Does this mean Firefox/Safari lack support or is there a different way we should write the test so that it passes in all browsers?

TimvdLippe · 2025-10-23T13:58:47Z

tests/wpt/tests/fetch/range/cache-revalidate-construct-range-response.html

+        const response_2 = await fetch(url.toString(), {
+            headers: {'Range': 'bytes=0-99'}
+        });
+        assert_equals(response_2.status, 206, 'Range fetch after cache revalidation should succeed with 206');


Firefox and Safari appear to return 200 instead: https://wpt.fyi/results/fetch/range/cache-revalidate-construct-range-response.html?pr=55613 This test should pass on those browsers as well. Can we update the test so that it correctly runs there and then update Servo to return the same results?

https://wpt.fyi/results/fetch/http-cache/partial.any.html?label=master&label=experimental&product=chrome&product=edge&product=firefox&product=safari&product=servo&aligned Seems like the problem is Firefox and safari would not tried to construct partial response from http_cache, hence not sending request with "IF-None-Match" or "If-Modified-Since" Headers. Given the scope of this test is to test whether UA could return partial request correctly if there are expired test in the http_cache, maybe i can modify the python to handle range request directly if received.

tests/wpt/tests/fetch/range/resources/video-with-aged-proxy-cache.py

TimvdLippe · 2025-10-23T14:03:38Z

tests/wpt/tests/fetch/range/cache-revalidate-construct-range-response.html

+function cache_revalidation_test(expect, label) {
+    promise_test(async t => {
+        const url = new URL('resources/video-with-aged-proxy-cache.py', location.href);
+        const response_1 = await fetch(url.toString());


Nit: string conversion isn't required for url when fetching

Suggested change

const response_1 = await fetch(url.toString());

const response_1 = await fetch(url);

TimvdLippe · 2025-10-23T14:03:54Z

tests/wpt/tests/fetch/range/cache-revalidate-construct-range-response.html

+        assert_equals(response_1.status, 200, 'Initial fetch should succeed with 200');
+        const response_1_blob = await response_1.blob();
+        assert_equals(response_1_blob.size, 80666, 'Initial fetch should get full content');
+        const response_2 = await fetch(url.toString(), {


Suggested change

const response_2 = await fetch(url.toString(), {

const response_2 = await fetch(url, {

tests/wpt/tests/fetch/range/cache-revalidate-construct-range-response.html

servo-wpt-sync · 2025-10-24T05:47:02Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#55613).

…red cache Signed-off-by: rayguo17 <[email protected]>

servo-wpt-sync · 2025-10-24T05:51:41Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#55613).

TimvdLippe

Looking at https://wpt.fyi/results/fetch/range/cache-revalidate-construct-range-response.html?label=pr_head&max-count=1&pr=55613 looks like it passes in all browsers. Thanks for fixing the issue and adding a test!

TimvdLippe · 2025-10-24T11:47:11Z

tests/wpt/tests/fetch/range/cache-revalidate-construct-range-response.html

+
+promise_test(async t => {
+    const url = new URL('resources/video-with-aged-proxy-cache.py', location.href);
+    const response_1 = await fetch(url.toString());


In my previous review I had a nit for this. Make sure to address review comments or acknowledge them if you disagree with them. This is a nit, so we can merge this now, but in the future please do ensure you address the full review.

My apology, I have changed it in local environment, may have lost it when I am switching develop environment.

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 22, 2025

rayguo17 marked this pull request as draft October 22, 2025 03:59

rayguo17 force-pushed the fix_http_cache branch 2 times, most recently from b3407cc to 2ac0ab6 Compare October 22, 2025 06:48

rayguo17 marked this pull request as ready for review October 22, 2025 06:49

TimvdLippe added the T-linux-wpt Do a try run of the WPT label Oct 22, 2025

github-actions bot removed the T-linux-wpt Do a try run of the WPT label Oct 22, 2025

TimvdLippe requested changes Oct 22, 2025

View reviewed changes

servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 22, 2025

jdm added S-needs-tests New tests have been requested by a reviewer. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Oct 22, 2025

fix http cache reconstruct response from cache after validation behav…

8bed438

…iour Signed-off-by: rayguo17 <[email protected]>

rayguo17 force-pushed the fix_http_cache branch from 2ac0ab6 to 8bed438 Compare October 23, 2025 11:59

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 23, 2025

servo-wpt-sync mentioned this pull request Oct 23, 2025

fix http cache reconstruct response from cache after validation behaviour web-platform-tests/wpt#55613

Merged

TimvdLippe requested changes Oct 23, 2025

View reviewed changes

fixing test for UA that will not construct partial response from expi…

d01266a

…red cache Signed-off-by: rayguo17 <[email protected]>

rayguo17 force-pushed the fix_http_cache branch from 35a0ea0 to d01266a Compare October 24, 2025 05:48

TimvdLippe approved these changes Oct 24, 2025

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 24, 2025

TimvdLippe added this pull request to the merge queue Oct 24, 2025

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 24, 2025

Merged via the queue into servo:main with commit 8f992ed Oct 24, 2025
30 checks passed

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 24, 2025

	const response_1 = await fetch(url.toString());
	const response_1 = await fetch(url);

	const response_2 = await fetch(url.toString(), {
	const response_2 = await fetch(url, {

Uh oh!

Conversation

rayguo17 commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yezhizhen commented Oct 22, 2025

Uh oh!

github-actions bot commented Oct 22, 2025

Uh oh!

github-actions bot commented Oct 22, 2025

Uh oh!

github-actions bot commented Oct 22, 2025

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

jdm commented Oct 22, 2025

Uh oh!

servo-wpt-sync commented Oct 23, 2025

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

TimvdLippe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

rayguo17 Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

TimvdLippe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

TimvdLippe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

servo-wpt-sync commented Oct 24, 2025

Uh oh!

servo-wpt-sync commented Oct 24, 2025

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

TimvdLippe Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

rayguo17 Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rayguo17 commented Oct 22, 2025 •

edited

Loading

rayguo17 Oct 24, 2025 •

edited

Loading