Fix: Prevent Errors in Header Processing and Encode URLs Properly

[Juzar10]

What?

Closes: #67358

This PR solves the issue with the non-ASCII characters in the slug of the CPT. when non-ASCII characters are present in the slug the Gutenberg editor displays error.

Why?

Attempting to edit the post of the CPT with the Persian characters results in a fatal error.

How?

this PR properly encodes the header value before sending the custom response from the api-fetch module. It solves the issue regarding the Gutenberg editor not working properly.

Testing Instructions

Steps

1. Add custom CPT with persian slug, can use the same as provided in the issue.
2. Create a new post of the CPT.
3. Attempt to edit the post of the CPT.

Before:

Screen.Recording.2024-12-10.at.1.15.08.PM.mov

After:

Screen.Recording.2024-12-10.at.1.13.04.PM.mov

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @mafiayemakhfi.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: mafiayemakhfi.

Co-authored-by: Juzar10 <[email protected]>
Co-authored-by: ellatrix <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: SainathPoojary <[email protected]>
Co-authored-by: himanshupathak95 <[email protected]>
Co-authored-by: t-hamano <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Juzar10! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[ellatrix]

What does this have to do with preloading? Does it work when you disable preloading altogether?

[Juzar10]

@ellatrix

Commenting out this line in apiFetch does seem to resolve the issue, though it results in the expected console error.

I suspect the issue might be related to incorrect encoding being sent in the headers. Please let me know if you'd like me to test anything further or if there are specific scenarios I should focus on.

Thanks!

[ellatrix]

Can you try making this empty instead?

gutenberg/packages/api-fetch/src/middlewares/preloading.js

Lines 11 to 16 in d9f18e5

	const cache = Object.fromEntries(
	Object.entries( preloadedData ).map( ( [ path, data ] ) => [
	normalizePath( path ),
	data,
	] )
	);

If that fixes it, then it's indeed related to preload I guess.

In that case, can you be a bit more descriptive on how the PR fixes the issue? What was the previous preloaded data and what is it after the fix? Are we adjusting the data at the right point? Why is the data fixed client side vs server side for example?

[Juzar10]

yes, putting the empty cache object does resolve the issue.

The issue stems from the inability to send non-ASCII characters in HTTP headers when using the Response constructor.

This PR addresses the problem by encoding non-ASCII characters in header links before sending them. Without this encoding, an error is thrown. ( you have to add catch block ingetEntityRecord function to check it )

TypeError: Response constructor: Cannot convert value in record<ByteString, ByteString> branch of (sequence<sequence<ByteString>> or record<ByteString, ByteString>) to ByteString because the character at index 23 has value 1608 which is greater than 255.
    prepareResponse http://localhost:8888/wp-content/plugins/gutenberg/build/api-fetch/index.min.js?ver=267f89098b708f8d02b1:680
    createPreloadingMiddleware http://localhost:8888/wp-content/plugins/gutenberg/build/api-fetch/index.min.js?ver=267f89098b708f8d02b1:637
    enhancedHandler http://localhost:8888/wp-content/plugins/gutenberg/build/api-fetch/index.min.js?ver=267f89098b708f8d02b1:178

Problematic Header Link:

Before encoding:

<http://localhost:8888/ویدیو/hello-video-world>; rel="alternate"; type=text/html

After encoding

<http://localhost:8888/%D9%88%DB%8C%D8%AF%DB%8C%D9%88/hello-video-world>; rel="alternate"; type=text/html

The fix involves encoding the header link just before it is sent, It won't affect other headers and ASCII characters.

Since the issue is from client side handling of the header link, particularly in the Gutenberg api-fetch module, it seems appropriate to address it here.

let me know if anything else needs to be tested regarding this or any other module I should look into.

Thanks.

[Mamaduka]

Thanks for contributing, @Juzar10!

I posted the latest update on the issue - #67358 (comment).

The current proposal always tries to encode Link headers, which is only needed when the middleware cannot wrap data in Response. Let's update the code and avoid unnecessary encoding. See the example patch below.

Can you also rebase your branch on top of the lastest trunk?

Patch

diff --git packages/api-fetch/src/middlewares/preloading.js packages/api-fetch/src/middlewares/preloading.js
index ac293738513..1401282d0e1 100644
--- packages/api-fetch/src/middlewares/preloading.js
+++ packages/api-fetch/src/middlewares/preloading.js
@@ -68,15 +68,37 @@ function createPreloadingMiddleware( preloadedData ) {
  * @return {Promise<any>} Promise with the response.
  */
 function prepareResponse( responseData, parse ) {
-	return Promise.resolve(
-		parse
-			? responseData.body
-			: new window.Response( JSON.stringify( responseData.body ), {
-					status: 200,
-					statusText: 'OK',
-					headers: responseData.headers,
-			  } )
-	);
+	if ( parse ) {
+		return Promise.resolve( responseData.body );
+	}
+
+	try {
+		return Promise.resolve(
+			new window.Response( JSON.stringify( responseData.body ), {
+				status: 200,
+				statusText: 'OK',
+				headers: responseData.headers,
+			} )
+		);
+	} catch {
+		Object.entries( responseData.headers ).forEach( ( [ key, value ] ) => {
+			if ( key.toLowerCase() === 'link' ) {
+				responseData.headers[ key ] = value.replace(
+					/<([^>]+)>/,
+					( /** @type {any} */ _, /** @type {string} */ url ) =>
+						`<${ encodeURI( url ) }>`
+				);
+			}
+		} );
+
+		return Promise.resolve(
+			new window.Response( JSON.stringify( responseData.body ), {
+				status: 200,
+				statusText: 'OK',
+				headers: responseData.headers,
+			} )
+		);
+	}
 }
 
 export default createPreloadingMiddleware;

[Juzar10]

@Mamaduka Thanks for the update regarding the issue.

I have applied the changes and the patch you suggested and have tested it. It is working.

Also updated the branch with latest trunk changes.

Thanks.

[Mamaduka]

Thank you, @Juzar10!

Do you mind adding a unit test for this scenario? Preloading middleware has good unit test coverage. We should also include this case.

[Juzar10]

@Mamaduka, Thanks.

I have updated the PR with the unit test regarding this scenario.

[Mamaduka]

Thank you, @Juzar10!