WP-API/node-wpapi

Wordpress 5.5 breaks discoverability

acburdine opened this issue ยท 11 comments

Ran into this after upgrading one of our wordpress sites to 5.5. This new version of wordpress adds a number of new API routes, including this one: /wp/v2/plugins/(?P<plugin>[^.\\/]+(?:\\/[^.\\/]+)?).

Currently, something in that regular expression is not playing nicely with the discoverability code, because whenever I point wpapi at our wordpress 5.5 site, I get the following error:

SyntaxError: Invalid regular expression: /^[^.\/]+(?:\/[^.\/]+$/: Unterminated group
    at new RegExp (<anonymous>)
    at reduceRouteComponents (./node_modules/wpapi/lib/route-tree.js:86:3)
    at Array.reduce (<anonymous>)
    at reduceRouteTree (./node_modules/wpapi/lib/route-tree.js:185:18)
    at Object.keys.reduce (./node_modules/wpapi/lib/util/object-reduce.js:25:20)
    at Array.reduce (<anonymous>)
    at module.exports (./node_modules/wpapi/lib/util/object-reduce.js:24:3)
    at buildRouteTree (./node_modules/wpapi/lib/route-tree.js:203:9)
    at WPAPI.bootstrap (./node_modules/wpapi/wpapi.js:349:23)
    at new WPAPI (./node_modules/wpapi/wpapi.js:88:4)

Was able to reproduce it with this test:

const wpapi = require('wpapi');

async function test() {
  await wpapi.discover('https://example-wordpress-55-site.com');
}

test().catch(console.error);

I added some logging in that reduceRouteComponents and confirmed that it's the plugins url that's messing things up.

Because discoverability is breaking, it falls back to assuming default routes, meaning none of our custom post types are currently usable via this package ๐Ÿ˜•

I'll do my best to investigate further and open a PR, but wanted to open this issue in case anyone else runs into the same problem.

Upon further investigation, it's this regex that's causing the issue, because the plugins regex has nested capture groups.

Not really sure how/if this is easily fixable - it might make more sense to just ignore routes with invalid regexes in the short-term ๐Ÿค”

msdsk commented

I replaced pattern from that file with const pattern = '\\(\\?(?:P<)(.+?)>(.+?)\\)(?:$|\\/)' - it seems to be matching well for me, but I'm using a very limited functionality so far and haven't tested it any further.

Created a temp fix branched off of v1, but maybe works for v2 as well PopularPays@ac9f187#diff-0d84322a522b9416af5ea995fa46d971R75-R83

If you're on v1 and want a patch you can reference it directly:

npm uninstall wpapi  && npm install npm install https://github.com/PopularPays/node-wpapi#pp-fix-regex --save

Couldn't download the patch listed above but I made the suggested changes which didn't work for me. Instead I've added these lines which resolved the problem and made autodiscover work with WP 5.5.1 by replacing the regex pattern, seams like a \ was missing.
Starting on Line 79 in route-tree.js
var myGroupPattern = groupPattern; if (myGroupPattern !== null) { myGroupPattern = myGroupPattern.replace("(?","\\(?"); } // console.log(myGroupPattern); const groupPatternRE = myGroupPattern === '' ? // If groupPattern is an empty string, accept any input without validation /.*/ : // Otherwise, validate against the group pattern or the component string new RegExp( myGroupPattern ? '^' + myGroupPattern + '$' : component, 'i' ); // Only one validate function is maintained for each node, because each node // is defined either by a string literal or by a specific regular expression. currentLevel.validate = input => groupPatternRE.test( input );

I have created a WordPress core trac ticket https://core.trac.wordpress.org/ticket/51467#ticket. The regex pattern that is in core now is valid PHP, but invalid for JavaScript.

There was a reply in the WordPress core tracker (https://core.trac.wordpress.org/ticket/51467#comment:2):

Testing just the plain pattern [^.\/]+(?:/[^.\/]+)? and ?[^.\/]+(?:/[^.\/]+)?). It appears to compile successfully for me in both Safari 14 and Node 12.

It looks like this might be an artifact of how node-wpapi handles transforming the route regex.

This, new RegExp(/^[^.\/]+(?:\/[^.\/]+$/); for instance should actually be new RegExp(/^[^.\/]+(?:\/[^.\/]+)?$/); which does compile successfully. Notice that the )? was dropped.

I write this regex \(\?(?:P<|<|')([^>']+)[>']([^\)]*(\))?)\??\) and I tested it ( very lightly with my local wordpress) and it seems to work.
if somene would publish a bugfix with that regex maybe after some more tests!
Thanks to all

I tested today and found that this bug breaks discoverability using the latest wpapi v1.2.1 and WP v5.7.

It looks like @socrates77 addressed this issue in a recent PR #479.

Can we help to prepare a patch release so this fix can be published to npm?

wpapi version 1.2.2 has been released, which includes @socrates77' change. Thank you for reporting this, and I am sorry to you all that I was lax in accepting and releasing this. I did adjust the RE slightly while rebasing that change into the v1 branch, so @kevinwhoffman @riddla @NielsdeBlaauw et al, if you still encounter this behavior after updating to v1.2.2, please @ me here with details.

@kadamwhite Thanks very much for the release. I successfully tested the following using wpapi 1.2.2 and WordPress 5.7.1:

WPAPI.discover(location.origin).then((response) => console.log(response));