Block theme with period in name causes blank editor

[mkaz]

Testing instructions:

1. Rename a block theme to include periods, for example: twentytwentytwo-1.0
2. Attempt to load site editor, confirm blank screen
3. Additional testing, add theme under subdirectory with or without periods

Original issue discovered in #54544

Upstream issue in Gutenberg plugin here:
​https://github.com/WordPress/gutenberg/issues/37156

The register rest routes are different between core and the Gutenberg plugin, this same change works in core but not in the plugin. So additional testing is needed to fix upstream.

[jffng]

I gave this patch a spin and confirm the site editor loads as expected when a theme name contains a period (e.g. twentytwentytwo-0.4.0), as well as when it appears in a subdirectory.

Fix: block theme with period in name causes blank editor.

Trac ticket: https://core.trac.wordpress.org/ticket/54596

[peterwilsoncc]

This looks to be a wider problem with other characters that are permitted in folder names but do not currently work. I've put some notes below.

Somewhat challenging is that not all characters allowed in a folder name are permitted in a post name, so global styles set up with the theme my-theme and my.theme may both end up with the same slug.

---

Manual testing with various characters in the folder name with this patch applied follows.

For the 2022 clone, success or failure is indicated by whether the editor works.

For the 2020 clone, success or failure is indicated by whether the customizer works.

Character	2022 clone	2020 clone
space	Failed	Successful
.	Successful	Successful
[	Failed	Failed
|	Failed	Successful
=	Failed	Successful
(	Failed	Successful
+	Failed	Successful
@	Failed	Successful

[mkaz]

@peterwilsoncc good catch. We can switch the regex to \w\W to grab everything, but not sure if that is a good idea or not, I fear spaces and other characters might introduce additional issues.

Your point about the slug is curious if themes share the same slug the styles and templates can conflict with one another.

[peterwilsoncc]

Yeah, I'm not sure what the correct answer to this is but I thought it better to note it down none the less.

The one thing that did occur to me overnight was that maybe the post slugs could be a hash of the directory name. It doesn't need to be salted as it's a hash of public information anyway. However, I suspect it's way to late in the release cycle to consider such a change.

@hellofromtonya I would love to see some unit test with a test data provider, with list of theme names with funny characters, like, {?: etc.

[hellofromTonya]

Replying to peterwilsoncc:

Character	2022 clone	2020 clone
space	Failed	Successful
.	Successful	Successful
[	Failed	Failed
|	Failed	Successful
=	Failed	Successful
(	Failed	Successful
+	Failed	Successful
@	Failed	Successful

After testing with @costdev, | fails on Windows as it's not a valid dirname and + gets stripped out.

​PR 2034 adds support for the following characters: ., [, ], (, ), @, _, and space.

It's ready for testing and review.

To test:

1. Pull and apply the PR.
2. Rename the twentytwentytwo theme's directory to something like twentytwentytwo -_@ [(v0.4.0)].
3. Activate the theme.
4. Click on the 'Customize' button.

Expected behavior: The Site Editor opens without issue.

I would love to see some unit test with a test data provider, with list of theme names with funny characters, like, {?: etc.

Me too, @spacedmonkey. Those data providers are there now.

[hellofromTonya]

Test Report: testing valid theme dirname with PR 2034 applied. Works as expected.

[hellofromTonya]

Test Report: testing invalid theme dirname with PR 2034 applied. Fails as expected.

[costdev]

Test Report

Env

• WordPress 5.9-beta2-52344-src
• Chrome 96.0.4664.93
• Windows 10
• Theme: Twenty Twenty-Two
• Gutenberg Editor
• Plugin: None activated.

Steps to test

1. Rename a block theme directory to include [ ] ( ) - _ (space) @. For example: [twenty_twenty-two] (@ 1.0).
2. Navigate to Appearance > Themes and activate the theme.
3. Navigate to Appearance > Site Editor.
4. The Site Editor should fail to load.
5. Apply ​PR 2034.
6. Repeat steps 1-3.
7. The Site Editor should now load.

Additional testing:

• Repeat steps 1-7 but place the theme in a sub-directory with [ ] ( ) - _ (space) @ in its directory name.

Results

• Before the PR: The Site Editor fails to load.
• After the PR: The Site Editor loads as expected.
• Issue fixed ✅

[hellofromTonya]

Test Report

Env:

• OS: macOS Big Sur 11.6
• Browsers: Edge, Chrome, Safari, and Firefox
• Theme: Copy of TT2 with directory name changed (see test info)
• Plugins: None
• WordPress: trunk
• Patch: PR 2034
• Localhost: wp-end and Local (with ngnix and apache)

Steps

1. Copy TT2 theme:
   • Rename it's directory to twentytwentytwo @[0.4.0] ( - _ - )
   • Open style.css and change the Theme Name
2. Go to Appearance > Themes
3. Activate the new TT2 theme with the funky directory naming
4. Go to the Site Editor, i.e. click on "Customize" button

Expected behavior: The Site Editor should load

5. Navigate back to the Themes UI
6. Activate the original TT2 theme
7. Rename the TT2 copy to twentytwentytwo {notvalid}
8. Activate this TT2
9. Click on "Customize" button

Expected behavior: The Site Editor should not load as there's an error with the invalid theme directory name

Results

• Confirmed using space, -, _, ., @, [, ], (, ) characters in dirname works ✅
• Confirmed using { and } does not work and gives a 404 error ✅

[hellofromTonya]

Marking ​PR 2034 for commit.

Note: This patch adds support for the following characters: ., [, ], (, ), _, and @. It does not add support for | (which fail on Windows), + (which gets stripped), or = (which is a separator for query string key=value pairs).

I would really like @TimothyBJacobs to review this before this is merged.

[hellofromTonya]

Removing the commit keyword until folks like @TimothyBlynJacobs and @mkaz review and/or test the PR.

[hellofromTonya]

Test: wp-content/themes/subdir/twentytwentytwo v0.4.0 @[( -_-)] (with latest PR changes). Works ✅

[justinahinon]

Test Report

Env:

• OS: macOS Big Sur 12.0
• Browsers: Firefox Developer Edition 95.0b12
• Theme: Copy of TT2 with directory name changed (see test info https://core.trac.wordpress.org/ticket/54596#comment:14)
• Plugins: None
• WordPress: trunk
• Patch: PR 2034
• Localhost: wp-env

Results

Before the patch

• Confirmed using space, -, _, ., @, [, ], (, ) characters in dirname works
• Confirmed using { and } does not work, and output a 404 error on http://localhost:8889/index.php?rest_route=/wp/v2/templates/twentytwentytwo {notvalid}//home&context=edit&_locale=user

[hellofromTonya]

Test Report: wp-content/themes/tt2-{invalid}/ (with latest PR changes). Result => status: 404, message: "No route was found matching the URL and request method"

[hellofromTonya]

Test Report: wp-content/themes/twentytwentytwo @[0.4.0] ( -_-)/ (with latest PR changes). Works ✅

[mkaz]

Test Report

Env

OS: Windows / WSL2 (Apache)

Theme: Twentytwentytwo

Plugins: None (Issue still present in Gutenberg plugin (See ​37167)

WordPress: trunk

Patch: PR 2034

Results

Works as expected, tested using set of characters directory name ( - _ . [ ] ( ) @ )

[hellofromTonya]

Marking it again for commit. If there are follow-ups from @TimothyBlynJacobs, can catch those in a follow-up commit for Beta 3 or Beta 4.

[TimothyBlynJacobs]

The change looks good to me. I do worry about @peterwilsoncc's concerns that different themes will end up mapping to the same slug.

@hellofromtonya This needs to be rebased and conflicts fixed.

[TimothyBlynJacobs]

Sorry noticed something, does this not affect the themes endpoint? Is that regex already broad enough?

[hellofromTonya]

Test Report: Do same theme with different directory names resolve correctly? Yes, works ✅

[hellofromTonya]

Test Report: 2 themes with same theme name but different directory names - Resolve in Site Editor? Yes, works ✅

[hellofromTonya]

Replying to TimothyBlynJacobs:

I do worry about @peterwilsoncc's concerns that different themes will end up mapping to the same slug.

I was worried about that too. Tested it with 2 the TT2 theme with different theme directory names and changed the wp-content/themes/twentytwentytwo-0.4.0/theme.json to background color to red. See attachments test-site-editor-same-theme-resolves.png​ and test-same-theme-slug-resolves.png​. Both themes resolve in the Site Editor, front-end, and in the Appearance > Themes.

Merge conflict resolved.

[hellofromTonya]

Replying to TimothyBlynJacobs:

Sorry noticed something, does this not affect the themes endpoint? Is that regex already broad enough?

Thanks for raising the question. Did further testing through Customizer, Site Editor, Block Editor, and frontend. I'm curiously not seeing an issue with the Themes endpoint.

[hellofromTonya]

In 52376:

REST API: Add block theme support for valid non-alphanumeric characters in theme's directory name.

Themes whose wp-content/themes/<dirname> include valid non-alphanumeric (cross-platform) characters work for non-block themes, but did not previously resolve for block themes. For example, a block theme in wp-content/themes/twentytwentytwo-0.4.0/ directory resulted a 404 "No route was found matching the URL and request method" response when attempting to customize it in the Site Editor.

This commit adds support for the following characters in a theme's root directory: _, ., @, [, ], (, and ). Subdirectory themes and - are already supported.

Follow-up to [51003], [52051], [52275].

Props mkaz, costdev, hellofromTonya, jffng, justinahinon, peterwilsoncc, spacedmonkey, TimothyBlynJacobs.
Fixes #54596.

Committed via https://core.trac.wordpress.org/changeset/52376.

[TimothyBlynJacobs]

Replying to hellofromTonya:

Replying to TimothyBlynJacobs:

Sorry noticed something, does this not affect the themes endpoint? Is that regex already broad enough?

Thanks for raising the question. Did further testing through Customizer, Site Editor, Block Editor, and frontend. I'm curiously not seeing an issue with the Themes endpoint.

The single /wp/v2/themes/<theme> endpoint would need to be tested manually or via Unit Tests. AFAIK, no parts of the UI currently use the single theme endpoint directly, they still use the collections response.

[hellofromTonya]

Replying to TimothyBlynJacobs:

The single /wp/v2/themes/<theme> endpoint would need to be tested manually or via Unit Tests. AFAIK, no parts of the UI currently use the single theme endpoint directly, they still use the collections response.

Thanks @TimothyBlynJacobs and DOH on my part :facepalm:. Reopening this ticket to test this, i.e. just to make sure that endpoint is okay or it needs an adjustment too.

Hey @costdev would you have some time this week to manually test this?

[bijayyadav]

I installed 5.3 beta and copied twentytwentytwo theme and renamed it twentytwentytwo!. After that on the backend, I clicked on Appearance > Editor and the page was blank. There are two errors on console:

• ​http://localhost/beta/wp-json/wp/v2/templates/twentytwentytwo!//home?context=edit&_locale=user 404 (Not Found)
• Uncaught (in promise) TypeError: Cannot destructure property 'id' of '(intermediate value)' as it is null. - ​http://localhost/beta/wp-includes/js/dist/edit-site.min.js?ver=dd4604f4d44cfa19261ecfc79f1e6f06

Screenshot: ​http://prntscr.com/2328a4g

[kafleg]

I also have the same issue. My theme folder is, twenty!@#$%^&()_+ ~-+`.

Also, there console error,

Uncaught (in promise) TypeError: Cannot destructure property 'id' of '(intermediate value)' as it is null.

at K (edit-site.min.js?ver=dd4604f4d44cfa19261ecfc79f1e6f06:2)
at K.next (<anonymous>)
at Y (edit-site.min.js?ver=dd4604f4d44cfa19261ecfc79f1e6f06:2)
at Y.next (<anonymous>)
at redux-routine.min.js?ver=f2016f404fbc2c831239953e37cc686c:2

[hellofromTonya]

Thanks @kafleg and @bijayyadav. Currently, the endpoint does not support these characters: !#$%^&+~.

TODO: Confirm if all of these characters are valid for directory names on cross-server OSs including Windows, Mac, Linux, etc.

Adds support for localized characters (e.g. é) and other valid directory name characters.

Trac ticket: https://core.trac.wordpress.org/ticket/54596

[hellofromTonya]

[52376] was too limiting as valid directory name characters such as é, &, =, #, !, $, ,, ^, ~, %, etc.

Notice that the original regex for listing themes global styles excludes . and / characters.

'/' . $this->rest_base . '/themes/(?P<stylesheet>[^.\/]+(?:\/[^.\/]+)?)'

Aha, . was being excluded.

Then @TimothyBlynJacobs asked if WP_REST_Themes_Controller::PATTERN was also too limiting:

Sorry noticed something, does this not affect the themes endpoint? Is that regex already broad enough?

Here's it original regex:

const PATTERN = '[^.\/]+(?:\/[^.\/]+)?';

Notice again the . character is excluded.

​PR 2064 reverts the regex changes made by [52376], removes the . from the exclusion lists, and adds additional invalid characters (invalid on Windows). It then adds additional tests to further validate the changes.

@costdev tested it on Windows. I tested it on Mac. Works well on both. Test report is coming.

[hellofromTonya]

Test PR 2064 (on macOS) with themes in wp-content/themes/subdir/tt2é [v1.0.0](-_-)/ and wp-content/themes/tt2é [v1.0.0] {@!$,^~}(-_-)/ directories. Works ✅

[hellofromTonya]

Test Report

Env:

• OS: macOS Big Sur 11.6
• Browsers: Edge
• Theme: Copies of TT2 with directory name changed (see test info)
• Plugins: None
• WordPress: trunk
• Patch: ​PR 2064
• Localhost: wp-env

Steps

1. Prepare first theme:
   • Copy wp-content/themes/twentytwentytwo/ to wp-content/themes/tt2é [v1.0.0] {@!$,^~}(-_-)/
   • In copied theme, open style.css and change the Theme Name to tt2é [v1.0.0] {@!$,^~}(-_-)
   • Open the theme.json and change the background color of #000000 to #FF0000 (visual cue that this theme resolves)
2. Prepare second theme:
   • Copy wp-content/themes/tt2é [v1.0.0] {@!$,^~}(-_-)/ to wp-content/themes/subdir/tt2é [v1.0.0](-_-)/
   • Open style.css and change the Theme Name to subdir/tt2é [v1.0.0](-_-)
   • Open the theme.json and change the background color of #FF0000 to #00FF00 (visual cue that this theme resolves)
3. Go to Appearance > Themes
4. Activate the subdir/tt2é [v1.0.0](-_-) theme (the one in the subdirectory)
5. Click its "Customize" button

Expected behavior: The Site Editor should load and the header background should be a bright green color.

5. Click on the W logo which opens the Site Editor's sidebar
6. Navigate to Templates

Expected behavior: The Added by column should list subdir/tt2é [v1.0.0](-_-). (See the left side of test-pr2064-macos.gif​.)

7. View the site in the frontend.
8. Repeat steps 4 through 7 with the wp-content/themes/tt2é [v1.0.0] {@!$,^~}(-_-)/ theme.

Expected behavior: The header background color should red and "Added by" column should list tt2é [v1.0.0] {@!$,^~}(-_-). (See the right side of test-pr2064-macos.gif​.)

9. Repeat steps 4 through 7 with the wp-content/themes/twentytwentytwo/ theme.

Expected behavior: The header background color should black and "Added by" column should list Twenty Twenty-Two.

Results

• Twenty Twenty-Two theme loads properly ✅
• wp-content/themes/tt2é [v1.0.0] {@!$,^~}(-_-)/ loads properly ✅
• wp-content/themes/subdir/tt2é [v1.0.0](-_-)/ loads properly ✅
• Copying TT2 wp-content/themes/sudir/tt2 loads properly ✅ (an extra test I did)

Works as expected ✅

Notes:

• On Mac, themes named with &, =, % characters do not load in the Site Editor. No 404 (so the route is found). Uncaught (in promise) TypeError: Cannot destructure property 'id' of '(intermediate value)' as it is null. error in the console. Suspect this is due to encoding.
  • Theme directories named with these characters are likely far edge cases.

[hellofromTonya]

Test (on Mac) BEFORE and AFTER PR 2064 with themes wp-content/themes/ŧĥémé-1.0.0/ and wp-content/themes/subdir/ŧĥémé-1.0.0/. Results: before fails; after works.

[hellofromTonya]

Theme directories named with characters like {@!$,^~"} and &=% are likely far edge cases. Theme directories named with localized characters like é are not.

Test Report

Env:

• OS: macOS Big Sur 11.6
• Browsers: Edge
• Theme: Copies of TT2 with directory name changed (see test info)
• Plugins: None
• WordPress: trunk
• Patch: ​PR 2064
• Localhost: wp-env

Steps

Using the steps as a starting point:

1. Prepare first theme:
   • Rename wp-content/themes/tt2é [v1.0.0] {@!$,^~}(-_-)/ to wp-content/themes/ŧĥémé-1.0.0/
   • Open style.css and change the Theme Name to ŧĥémé-1.0.0
2. Prepare second theme:
   • Rename wp-content/themes/subdir/tt2é [v1.0.0](-_-)/ to wp-content/themes/subdir/ŧĥémé-1.0.0
   • Open style.css and change the Theme Name to subdir/ŧĥémé-1.0.0

Now test:

1. Use trunk (not the PR)
2. Go to Appearance > Themes
3. Activate the subdir/ŧĥémé-1.0.0 theme (the one in the subdirectory)
4. Click its "Customize" button

Actual behavior: Site Editor does not load and has a 404 with the following error message: {"code":"rest_no_route","message":"No route was found matching the URL and request method.","data":{"status":404}} (To view the error, open browser's Network tab, refresh the page, scroll down the 404 item in the list, click on it to open a panel, and then click on the Response tab to view the JSON response.)

5. Apply PR 2064
6. Refresh the Site Editor.

Expected behavior: The Site Editor should load and the header background should be a bright green color.

Results

• Yes, can reproduce the 404 error before the PR is applied ✅
• With the PR 2064 applied, resolves the endpoint (no error) and Site Editor opens as expected ✅

Marking for commit.

[hellofromTonya]

In 52399:

REST API: Support . in theme directory names in WP_REST_Global_Styles_Controller, WP_REST_Templates_Controller, and WP_REST_Themes_Controller.

Regex changes from [52376] are reverted to restore the original regex patterns. Why? [52376] used an include characters pattern, which was too limiting. It did not account for localized characters, such as é, or other valid directory name characters.

The original theme directory regex pattern, i.e. [^.\/]+(?:\/[^.\/]+)? excluded the period . character. Removing the . character resolves the reported issue by allowing matching for themes/theme-dirname-1.0/ or themes/<subdirname>/theme-dirname-1.0/.

As the pattern used an exclude approach, all characters are valid for matching except for /. However, not all characters are cross-platform valid for directory names. For example, the characters /:<>*?"| are not valid on Windows OS. The pattern now excludes those characters.

The theme's directory (or subdirectory) name pattern matching is now used in WP_REST_Global_Styles_Controller, WP_REST_Templates_Controller, and WP_REST_Themes_Controller.

Follow-up to [51003], [52051], [52275], [52376].

Props costdev, hellofromTonya, spacedmonkey, TimothyBlynJacobs, bijayyadav, kafleg.
Fixes #54596.

Committed via changeset https://core.trac.wordpress.org/changeset/52399.