Fix Safari failing to make cross-origin HTTP requests by ashfame · Pull Request #3440 · WordPress/wordpress-playground

ashfame · 2026-03-28T04:31:40Z

Motivation for the change, related issues

Problem:
In Safari, WordPress beta tester plugin fails to offer the correct updates that are available after updating the channels to Beta/RCs in its settings. This happen because, the request to api.wordpress.org doesn't go through.

Root cause:
Safari does not support ReadableStream as a fetch() request body, throwing "NotSupportedError: ReadableStream uploading is not supported". fetchWithCorsProxy used ReadableStream.tee() to duplicate the body for the direct-fetch + CORS-proxy-fallback pattern, which broke all POST requests (e.g. wp_version_check to api.wordpress.org) in Safari.

Implementation details

For browsers that support it, keep the original streaming teeRequest() direct-fetch + CORS-proxy-fallback flow. For Safari (no ReadableStream upload support), buffer the request body into an ArrayBuffer so it can be reused for both the direct fetch attempt and the proxy retry.

The issue is confirmed to fix with this PR, but the issue only surfaces locally when ran like the following:

npx nx build playground-website
cd dist/packages/playground/wasm-wordpress-net
php -S 127.0.0.1:7777 -t .

The issue doesn't surface when running npm run dev, not sure why.

Testing Instructions (or ideally a Blueprint)

Checkout this branch locally & run with:

npx nx build playground-website
cd dist/packages/playground/wasm-wordpress-net
php -S 127.0.0.1:7777 -t .

Open http://localhost:7777/?plugin=wordpress-beta-tester in Safari browser.
Go to Dashboard > Updates, only Nightly is offered.
Go to Tools > Beta Testing
Select Bleeding-edge and then Beta/RC, save.
Now go to Dashboard > Updates, 7.0-RC2 will be offered for upgrade.

adamziel · 2026-03-28T09:00:44Z

Let's still use the body stream when supported, no need to make it worse for other browsers.

adamziel · 2026-03-28T09:02:34Z

Alternatively, we could try to emulate stream support using XHR. This matters when uploading big files.

brandonpayton · 2026-03-28T17:26:03Z

+	 * attempt and the CORS proxy fallback. We buffer into an ArrayBuffer
+	 * instead of using ReadableStream.tee() because Safari does not support
+	 * ReadableStream as a fetch() request body ("ReadableStream uploading
+	 * is not supported").


Fortunately, it looks like "ReadableStream request bodies" are part the WebKit Interop 2026 plans:
https://webkit.org/blog/17818/announcing-interop-2026/#fetch-uploads-and-ranges

ReadableStream request bodies enable true streaming uploads, letting you upload large files or real-time data without loading everything into memory first:

Safari does not support ReadableStream as a fetch() request body, throwing "NotSupportedError: ReadableStream uploading is not supported". fetchWithCorsProxy used ReadableStream.tee() to duplicate the body for the direct-fetch + CORS-proxy-fallback pattern, which broke all POST requests (e.g. wp_version_check to api.wordpress.org) in Safari. Buffer the request body into an ArrayBuffer instead so it can be reused for both fetch attempts. Made-with: Cursor

…pport Instead of unconditionally buffering the request body into an ArrayBuffer for all browsers, detect at runtime whether the browser supports ReadableStream as a fetch() request body. Use the streaming teeRequest() approach when supported (Chrome, Firefox) and fall back to ArrayBuffer buffering only when not (Safari). Addresses review feedback to avoid degrading streaming performance for browsers that already support it. Made-with: Cursor

Add a beforeAll hook that triggers the one-time ReadableStream body support detection before any per-test fetch mocks are installed, so the detection's internal fetch() call doesn't consume mock responses. Made-with: Cursor

Copilot

Pull request overview

This PR addresses Safari failures for cross-origin POST requests by avoiding ReadableStream request bodies when unsupported, enabling the direct-fetch + CORS-proxy-fallback flow to work in Safari.

Changes:

Add runtime feature detection for ReadableStream as a fetch() request body and cache the result.
For non-supporting browsers, buffer the request body into an ArrayBuffer to reuse it across direct fetch and proxy retry.
Update tests to “pre-warm” the feature detection cache to avoid consuming per-test mocked fetch responses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
packages/php-wasm/web-service-worker/src/fetch-with-cors-proxy.ts	Adds `ReadableStream` upload support detection and an `ArrayBuffer` buffering fallback for Safari before retrying via CORS proxy.
packages/php-wasm/web-service-worker/src/fetch-with-cors-proxy.spec.ts	Adds suite-level warmup to keep feature detection from interfering with per-test `fetch` mocks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

After teeRequest/buffering, requestObject's body stream may be locked. Defensively use directRequest (which carries identical metadata) in the catch block to avoid relying on the original request's body state. Made-with: Cursor

Export a test-only resetStreamBodySupportedForTesting() function so the cached feature detection can be cleared between test runs. Add three tests that force the non-streaming path by rejecting the detection probe, covering buffered POST bodies for both the direct fetch and CORS proxy fallback, as well as bodyless GET requests. Made-with: Cursor

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Reduces public API surface by grouping test utilities under a clearly internal __testing export object. Made-with: Cursor

brandonpayton · 2026-03-31T23:34:43Z

+				controller.close();
+			},
+		});
+		await fetch('data:a/a,', {


Is there a reason we need to use a meaningless MIME type here? If so, let's note why in a comment. If not, maybe we can remove the type altogether. The data URL spec says it is totally optional, and fetch('data:,') works fine in Firefox, Chrome, and Safari for me.

You are right. I didn't realise mime type was optional. Good to know. Tested 👍

@ashfame I didn't know/remember it was optional either, before looking at this case.

brandonpayton · 2026-03-31T23:48:10Z

+		 * fetch attempt and the CORS proxy fallback. Safari does not
+		 * support ReadableStream as a fetch() request body
+		 * ("ReadableStream uploading is not supported").


Wherever we note this, let's:

prefix the comment about Safari with a date

clarify to "does not currently support"

mention that Safari support is in the works for Interop 2026

include a reference link: https://web.dev/blog/interop-2026#fetch_uploads_and_ranges

This way we can feel comfortable removing the workaround when it is no longer needed.

brandonpayton · 2026-03-31T23:59:48Z

+		const [teedDirect, teedProxy] = await teeRequest(requestObject);
+		directRequest = teedDirect;
+		corsProxyBody = teedProxy.body;
+	} else {


Is there a reason we shouldn't move this if/else into the teeRequest() utility?

If we solve this problem in the utility function, it will be solved wherever teeRequest() is used. teeRequest() is not currently used elsewhere in this repo, but I think the workaround belongs there, unless there is a reason not to move it.

Very good point! I can refactor it in a follow up PR so that the fix is not still waiting to land. Sounds good?

brandonpayton · 2026-04-01T00:10:33Z

+			const rootUrl = new URL(import.meta.url);
+			rootUrl.pathname = '';
+			rootUrl.search = '';
+			rootUrl.hash = '';
+			const corsProxyUrlObj = new URL(corsProxyUrl, rootUrl.toString());


Why do we need to clear all the fields on the rootUrl? Wouldn't the URL() constructor be able to make sense of the root URL on its own?

For example, in the browser dev tools console, new URL('a', new URL('http://happycode.net/?stuff#huzzah')).href evaluates to 'http://happycode.net/a'.

I think this can be as simple as:

if ( useStreaming && body && new URL(corsProxyUrlObj, import.meta.url).protocol === 'http:' ) {

I know the rootUrl changes are pre-existing code that was moved. But I think we can either simplify while we're moving around what I think is unnecessary code, or maybe we should just leave the code alone except for adding useStreaming to the original if.

There may be something I'm missing, but that is my read here.

Yeah, seems alright to do. I have made the change.

…on works fine without it

Centralize ReadableStream upload capability probing and body preparation in a reusable utility so callers get Safari buffering behavior automatically. Update CORS proxy and tests to assert streaming vs buffered branches explicitly. Made-with: Cursor

Align fetch-with-cors-proxy test names and comments with the new prepareRequestForRetry flow so they no longer refer to removed teeRequest/duplexSafeFetch internals. Made-with: Cursor

ashfame · 2026-04-06T09:03:16Z

@brandonpayton Suggested refactoring done, ready for re-review.

brandonpayton

@ashfame I don't understand why we are getting rid of the teeRequest() util here and left a question about that.

brandonpayton · 2026-04-10T02:28:46Z

+		directRequest,
+		retryBody: corsProxyBody,
+		useStreamingBody,
+	} = await prepareRequestForRetry(requestObject);


Why are we replacing teeRequest() here?

If we are worried about unnecessarily duplicating a buffer in the streaming case, could we make a request object with a property getter that duplicates it on demand?

I think you might be suggesting wrapping the tee'd result in a request-like object with a getter for the retry body? The tee/buffer still has to happen eagerly before the first fetch() consumes the stream, so the getter wouldn't defer any work — it'd just be a different way to access the same pre-computed value. We also need useStreamingBody surfaced for the HTTP/1.1 CORS proxy buffering (lines 104-111), which depends on corsProxyUrl the caller has.

Let me know if I'm misreading your suggestion though!

@ashfame @adamziel is there any reason we couldn't just use Request#clone() here instead of our own teeRequest() utility (or utili-tee)?

From the MDN docs:

In fact, the main reason clone() exists is to allow multiple uses of body objects (when they are one-use only.)

It sounds like this should cover both cases, browsers that support streamed body properties and those that do not.

Is there a catch? If not, would this problem be solved if we just used that instead?

@brandonpayton Safari would still not support sending ReadableStream as a fetch() req body. So the fix would still be required. If you are wondering about just swapping the underlying utility, we can't because we need to modify the url and body of the request, which clone() would not let us do.

Yeah, I'm sorry, @ashfame. You're right that Request objects are immutable, and we need to create another Request with the CORS proxy URL. I should have noticed. My earlier suggestion about moving the logic into teeRequest() also seems misguided because I was only thinking about cloning the body, not about how we'd need to create a whole new Request to target a different URL.

I just have a feeling this could be much simpler and clearer.

Unfortunately, browser support here is a bit odd. Neither Safari nor Firefox support streaming Request bodies, but what they support is inconsistent between them.

Safari

Even though Safari doesn't support passing a ReadableStream as a body to fetch(), the "fetch" events I see in the Safari Service Worker do have a body property that is a ReadableStream.

As a test in Safari because it was near at hand, I made this request in the browser dev tools :
r = new Request('https://brandonpayton.github.io/wasm-posix-kernel/', { method: 'POST', body: new ArrayBuffer(100) })

The type of r.body was ReadableStream.

Then I opened the service worker dev tools, set a breakpoint on the fetch event, switched back to the original dev tools, and called fetch(r).

The request received by Service Worker also has a body that is a ReadableStream.

You can also create a request like:
r = new Request('https://brandonpayton.github.io/wasm-posix-kernel/', { method: 'POST', body: stream, duplex: 'half' })

But if you try to pass it to fetch() there is an "Unhandled Promise Rejection: NotSupportedError: ReadableStream uploading is not supported" error.

Firefox

In Firefox, with
r = new Request('https://brandonpayton.github.io/wasm-posix-kernel/', { method: 'POST', body: new ArrayBuffer(100) }), r.body doesn't seem to exist at all, so we cannot use things like `new Response(request.body).arrayBuffer().

What we can use for both Firefox and Safari to get access to a request's body is r.arrayBuffer().

How to support Chrome, Safari, and Firefox

It seems like the main things we need to make are:

Find out whether the browser supports passing a ReadableStream request body to fetch(), as this PR is already doing.

Use request.clone() to make have a second request for retry. This should cover all browsers, either teeing a ReadableStream body or duplicating a body buffer for use in a second possible request.

If we can use streaming request body, just pass along clonedRequest.body to the second fetch() and set the duplex: 'half' property.

If we can't use streaming request body, get the body as an ArrayBuffer via clonedRequest.arrayBuffer() and pass that as the body.

We cannot use new Response(clonedRequest.body).arrayBuffer() because Firefox does not expose a clonedRequest.body property today.

Note: AFAICT, it can be tricky to answer whether a request had a body to begin with. Perhaps if the buffer is zero-length and there is no content-type header, we can ignore the body altogether.

I think this can be done without teeRequest() or the shared preparation utility (which I originally implicitly encouraged by asking why more shouldn't be done within teeRequest()). I think doing it all here in sequence is probably clearer than having a prepare-for-retry helper.

What do you think?

Spot on! Much simpler implementation now. Thanks for being so meticulous!

…browser retry The prepareRequestForRetry utility relied on request.body (a ReadableStream) to detect and handle request bodies, which breaks on Firefox (where request.body is undefined) and Safari (where ReadableStream uploads throw). Inline the simpler cross-browser approach directly in fetchWithCorsProxy: clone the request before the first fetch, then extract the body from the clone for retry via arrayBuffer() (Safari/Firefox) or ReadableStream (Chrome). Made-with: Cursor

brandonpayton

Thanks for working on this, @ashfame!

I think the tests need some work in a follow-up PR. I left a note about that. But this looks like a good change. It's simple to reason about and looks like it solves the issue.

brandonpayton · 2026-04-15T05:04:22Z

+	 */
+	describe('non-streaming fallback (Safari/Firefox)', () => {
+		beforeEach(() => {
+			__testing.resetStreamBodySupported();


I'm concerned that we are just assuming/implying streaming is detected as unsupported due to mocks. The tests would be stronger if they positively assert that streaming is detected as unsupported. And since we have access to the utils here, that is possible.

Let's move the detection and mocking into the the beforeEach() so each of these tests asserts that streaming is unsupported prior to running.

brandonpayton · 2026-04-15T05:05:08Z

+		it('handles GET requests with no body in non-streaming mode', async () => {
+			const fetchMock = vi
+				.spyOn(globalThis, 'fetch')
+				.mockResolvedValue(new Response('ok'));


How is this in non-streaming mode if we are doing this above?

beforeEach(() => { __testing.resetStreamBodySupported(); });

We aren't mocking anything for streaming detection, and if the tests are running on Node.js / V8, won't this mean that this test runs when streaming body is detected as supported?

Note: this comment becomes irrelevant if we make force a streaming-body-unsupported condition in the beforeEach().

Follow-up to #3440. The non-streaming fallback tests relied on implicit mock ordering to simulate an unsupported streaming environment. On Node.js/V8 (where streaming IS supported), the GET test was not actually exercising the non-streaming path. Force `supportsReadableStreamBody()` to return `false` via a new `__testing.setStreamBodySupported()` setter and assert it in beforeEach, so every test positively confirms it runs in non-streaming mode. Made-with: Cursor

## Summary Follow-up to #3440, specifically addressing Brandon's comments [the request to assert unsupported streaming in each fallback test](#3440 (comment)) and [the concern that resetting the cache allows Node.js/V8 to detect streaming support](#3440 (comment)). - The non-streaming fallback suite now sets the cached `supportsReadableStreamBody()` result to `false` in `beforeEach()` and asserts that value before every test runs. - The suite resets that test-only cache in `afterEach()`, keeping the test state hermetic. - The proxy-retry test now correctly expects only direct-fetch and proxy-fetch calls, because no detection probe runs when unsupported mode is already cached. ## Test plan - [x] `npm exec nx test php-wasm-web-service-worker --testFile=fetch-with-cors-proxy.spec.ts` - [x] `npm exec nx lint php-wasm-web-service-worker`

ashfame self-assigned this Mar 28, 2026

brandonpayton reviewed Mar 28, 2026

View reviewed changes

ashfame mentioned this pull request Mar 30, 2026

Serve client on 127.0.0.1:5400/client/index.js when running npm run dev #3302

Closed

brandonpayton added [Type] Bug An existing feature does not function as intended [Priority] High labels Mar 31, 2026

ashfame added 2 commits March 31, 2026 21:18

ashfame force-pushed the beta_tester_safari_prod_only branch from b427a3f to 4367a2f Compare March 31, 2026 15:33

ashfame added 2 commits March 31, 2026 22:27

Pre-warm stream body feature detection in tests

28c4044

Add a beforeAll hook that triggers the one-time ReadableStream body support detection before any per-test fetch mocks are installed, so the detection's internal fetch() call doesn't consume mock responses. Made-with: Cursor

add back code comments

0e6f502

ashfame marked this pull request as ready for review March 31, 2026 18:43

ashfame requested review from a team, Copilot and zaerl March 31, 2026 18:43

Copilot AI reviewed Mar 31, 2026

View reviewed changes

ashfame and others added 4 commits April 1, 2026 01:54

Update packages/php-wasm/web-service-worker/src/fetch-with-cors-proxy.ts

ade6477

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'trunk' into beta_tester_safari_prod_only

a4d807e

ashfame requested a review from Copilot March 31, 2026 20:25

Copilot AI reviewed Mar 31, 2026

View reviewed changes

Move test-only reset function under __testing namespace

d79d665

Reduces public API surface by grouping test utilities under a clearly internal __testing export object. Made-with: Cursor

ashfame requested a review from brandonpayton March 31, 2026 20:39

brandonpayton reviewed Apr 1, 2026

View reviewed changes

ashfame and others added 4 commits April 3, 2026 12:18

remove mime type from the data url since its optional and our detecti…

f6176bc

…on works fine without it

clarify safari support comment better

2499f16

simplify check

b84d8da

Merge branch 'trunk' into beta_tester_safari_prod_only

5391a96

ashfame added 4 commits April 6, 2026 14:37

clean up stale test wording after retry utility refactor

de69cb8

Align fetch-with-cors-proxy test names and comments with the new prepareRequestForRetry flow so they no longer refer to removed teeRequest/duplexSafeFetch internals. Made-with: Cursor

add clarifying code comments

ac2f483

fix comment

0a5ac58

ashfame requested a review from brandonpayton April 6, 2026 09:03

brandonpayton requested changes Apr 10, 2026

View reviewed changes

brandonpayton approved these changes Apr 15, 2026

View reviewed changes

brandonpayton merged commit f826594 into trunk Apr 15, 2026
47 checks passed

brandonpayton deleted the beta_tester_safari_prod_only branch April 15, 2026 05:07

ashfame mentioned this pull request Apr 15, 2026

Harden non-streaming fallback tests for fetchWithCorsProxy #3495

Merged

2 tasks

Uh oh!

Conversation

ashfame commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for the change, related issues

Implementation details

Testing Instructions (or ideally a Blueprint)

adamziel commented Mar 28, 2026

adamziel commented Mar 28, 2026

brandonpayton Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashfame commented Apr 6, 2026

brandonpayton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Safari

Firefox

How to support Chrome, Safari, and Firefox

Choose a reason for hiding this comment

brandonpayton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Labels

4 participants

ashfame commented Mar 28, 2026 •

edited

Loading

brandonpayton Mar 28, 2026 •

edited

Loading