diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt index 389e4a912..db1933797 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -336,10 +336,9 @@ internal class MediaUploadServer( * Relays the editor's orphan cleanup to WordPress. * * Core's media upload middleware deletes the attachment when every - * `post-process` retry fails. A cross-origin editor cannot issue that - * request directly — api-fetch tunnels `DELETE` as a `POST` carrying - * `X-HTTP-Method-Override`, which core's CORS allow-list omits, so the - * browser blocks it at preflight. Relaying it here lets the cleanup run. + * `post-process` retry fails. The editor can now send that request directly, + * even cross-origin, so this route may no longer be needed. See + * https://github.com/wordpress-mobile/GutenbergKit/issues/860. */ private suspend fun handleDelete(attachmentId: String, query: String): HttpResponse { val client = internalClient ?: return errorResponse(500, "No internal media client configured") diff --git a/src/utils/api-fetch-upload-middleware.test.js b/src/utils/api-fetch-upload-middleware.test.js index 4f954afcd..32238f4d7 100644 --- a/src/utils/api-fetch-upload-middleware.test.js +++ b/src/utils/api-fetch-upload-middleware.test.js @@ -528,9 +528,7 @@ describe( 'nativeMediaUploadMiddleware', () => { } ); it( 'routes an attachment deletion through the native server', async () => { - // api-fetch tunnels DELETE as POST + `X-HTTP-Method-Override`, which - // core's CORS allow-list omits, so a cross-origin editor cannot make - // this request directly. Relaying it keeps the orphan cleanup working. + // The native server relays the deletion, as it does the upload. global.fetch = vi.fn( () => Promise.resolve( new Response( '{"deleted":true}', { status: 200 } ) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index c60044365..dae0b958d 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -37,6 +37,9 @@ const RETRY_DELAYS_MS = [ 500, 2000 ]; /** Longest `Retry-After` delay worth waiting for; longer ones fail at once. */ const MAX_RETRY_AFTER_MS = 10_000; +/** Header api-fetch's `httpV1Middleware` sets when sending a write as POST. */ +const METHOD_OVERRIDE_HEADER = 'X-HTTP-Method-Override'; + /** * Initializes the API fetch configuration and middleware. * @@ -93,7 +96,9 @@ export function configureApiFetch() { // Registered last so it runs first, covering preloaded responses too. apiFetch.use( persistedPostOverlayMiddleware ); apiFetch.setFetchHandler( - withRateLimitRetry( apiFetch.defaultFetchHandler ) + withRateLimitRetry( + withCrossOriginRealMethod( apiFetch.defaultFetchHandler ) + ) ); } @@ -645,12 +650,8 @@ function rejectUnreachableUpload( connectionError, options ) { * through to its normal handling. * * Core's media upload middleware deletes the orphaned attachment when every - * `post-process` retry fails. That request cannot be made directly from a - * cross-origin editor: `@wordpress/api-fetch` tunnels `DELETE` as a `POST` - * carrying `X-HTTP-Method-Override`, and core's `rest_allowed_cors_headers` - * does not list that header, so the browser blocks it at preflight and the - * orphan survives. Relaying through native code — which sets its own CORS - * policy — is what lets the cleanup complete. + * `post-process` retry fails. Native code relays that deletion, as it does the + * upload. * * This runs before `X-HTTP-Method-Override` exists: api-fetch's `httpV1` * middleware adds it further down the chain, so the method here is still a @@ -665,6 +666,10 @@ function rejectUnreachableUpload( connectionError, options ) { * @param {NativeUploadTransport} transport How to reach native code. * @param {(options: Object) => Promise} next The next middleware, for a refused deletion. * @return {?Promise} The relayed deletion, or `null` if not applicable. + * + * @todo Consider removing, as `withCrossOriginRealMethod` lets the editor send + * this deletion directly, even cross-origin. See + * https://github.com/wordpress-mobile/GutenbergKit/issues/860. */ function nativeMediaDelete( options, transport, next ) { if ( options.method?.toUpperCase() !== 'DELETE' || ! options.path ) { @@ -1313,6 +1318,60 @@ async function parseResponse( response, isOk ) { return body; } +/** + * Wraps a fetch handler to send cross-origin writes with their real method, as + * WordPress's CORS preflight rejects api-fetch's `X-HTTP-Method-Override`. The + * `_method` query parameter fails too: WordPress applies it to the preflight, + * which carries no credentials. + * + * Exported for testing only. + * + * @param {FetchHandler} fetchHandler The handler performing the request. + * @return {FetchHandler} The handler with cross-origin methods restored. + */ +export function withCrossOriginRealMethod( fetchHandler ) { + return ( options ) => { + const method = options.headers?.[ METHOD_OVERRIDE_HEADER ]; + if ( + ! method || + ! options.url || + ! isCrossOriginFromHttpPage( options.url ) + ) { + return fetchHandler( options ); + } + + const { [ METHOD_OVERRIDE_HEADER ]: _header, ...headers } = + options.headers; + // `fetch` uppercases `put` and `delete` but not `patch`, and the + // preflight compares methods case-sensitively. + return fetchHandler( { + ...options, + method: method.toUpperCase(), + headers, + } ); + }; +} + +/** + * Whether a request from this page to the URL is cross-origin under CORS; + * `file:` pages, like the iOS editor's, are exempt. + * + * @param {string} url The request URL. + * @return {boolean} True when the page is HTTP(S) and the URL is cross-origin. + */ +function isCrossOriginFromHttpPage( url ) { + const { protocol, origin, href } = window.location; + if ( protocol !== 'http:' && protocol !== 'https:' ) { + return false; + } + + try { + return new URL( url, href ).origin !== origin; + } catch { + return false; + } +} + const defaultPreloadData = { [ POST_TYPES_PATH ]: { body: { diff --git a/src/utils/api-fetch.test.js b/src/utils/api-fetch.test.js index e6dacba6c..9f6d77b88 100644 --- a/src/utils/api-fetch.test.js +++ b/src/utils/api-fetch.test.js @@ -13,6 +13,7 @@ import { configureApiFetch, persistedPostOverlayMiddleware, postTypesFallbackMiddleware, + withCrossOriginRealMethod, withEditedPostType, withRateLimitRetry, } from './api-fetch'; @@ -1028,6 +1029,100 @@ describe( 'api-fetch credentials handling', () => { } ); } ); + describe( 'withCrossOriginRealMethod', () => { + // The editor page is `http://localhost:3000` in jsdom, so the configured + // `https://example.com` root is cross-origin. + beforeEach( () => { + bridge.getGBKit.mockReturnValue( { + siteApiRoot: 'https://example.com/wp-json/', + siteApiNamespace: [], + namespaceExcludedPaths: [], + } ); + } ); + + const overriddenPut = ( url ) => ( { + url, + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-HTTP-Method-Override': 'PUT', + }, + } ); + + it.each( [ 'PUT', 'PATCH', 'DELETE' ] )( + 'sends a cross-origin %s with its real method', + async ( method ) => { + await apiFetch( { path: '/wp/v2/posts/1', method } ); + + const [ url, options ] = global.fetch.mock.calls[ 0 ]; + expect( new URL( url ).pathname ).toBe( + '/wp-json/wp/v2/posts/1' + ); + expect( options.method ).toBe( method ); + expect( options.headers ).not.toHaveProperty( + 'X-HTTP-Method-Override' + ); + expect( options.headers[ 'Content-Type' ] ).toBe( + 'application/json' + ); + } + ); + + it.each( [ 'put', 'patch', 'delete' ] )( + 'uppercases a cross-origin %s method', + async ( method ) => { + await apiFetch( { path: '/wp/v2/posts/1', method } ); + + const [ , options ] = global.fetch.mock.calls[ 0 ]; + expect( options.method ).toBe( method.toUpperCase() ); + } + ); + + it( 'keeps the header on a same-origin request', () => { + const fetchHandler = vi.fn(); + const handler = withCrossOriginRealMethod( fetchHandler ); + const options = overriddenPut( + `${ window.location.origin }/wp-json/wp/v2/posts/1` + ); + + handler( options ); + + expect( fetchHandler ).toHaveBeenCalledWith( options ); + } ); + + it.each( [ 'GET', 'POST' ] )( + 'leaves a cross-origin %s request unchanged', + async ( method ) => { + await apiFetch( { path: '/wp/v2/posts', method } ); + + const [ , options ] = global.fetch.mock.calls[ 0 ]; + expect( options.method ).toBe( method ); + expect( options.headers ).not.toHaveProperty( + 'X-HTTP-Method-Override' + ); + } + ); + + it( 'keeps the header when the page is not served over HTTP', () => { + const fetchHandler = vi.fn(); + const handler = withCrossOriginRealMethod( fetchHandler ); + const options = overriddenPut( + 'https://example.com/wp-json/wp/v2/posts/1' + ); + vi.spyOn( window, 'location', 'get' ).mockReturnValue( + new URL( 'file:///editor/index.html' ) + ); + + try { + handler( options ); + } finally { + vi.restoreAllMocks(); + } + + expect( fetchHandler ).toHaveBeenCalledWith( options ); + } ); + } ); + it( 'should preserve other headers when adding Authorization', async () => { bridge.getGBKit.mockReturnValue( { siteApiRoot: 'https://example.com/wp-json/',