Registering rest routes with a slash-prefixed namespace give inconsistent results

[skarabeq]

In the custom API when I use the prefix wp, I have got the error message - No route was found matching the URL and request method

{
    "code": "rest_no_route",
    "message": "No route was found matching the URL and request method",
    "data": {
        "status": 404
    }
}

In the last stable WordPress version 5.3.2, it wasn't happening!

[ocean90]

Might be related to #48530.

[TimothyBlynJacobs]

Hi @skarabeq,

Can you share your register_rest_route call?

[skarabeq]

Yes, of course

<?php
$model = new Model;
register_rest_route('/wp/v2', '/nav/' ,
    [
        [
            'methods'  => 'POST',
            'callback' => [$model, 'get_data'],
        ],
        'schema' => null,
    ]
);

[TimothyBlynJacobs]

Thanks @skarabeq,

The issue here is that you are registering your route with an invalid namespace. The namespace must not be prefixed by a forward slash. This configuration works.

<?php
register_rest_route(
        'wp/v2',
        '/nav/',
        array(
                'methods'  => 'GET',
                'callback' => static function () {
                        return new WP_REST_Response( null, 204 );
                },
        )
);

Now, register_rest_route sanitizes this malformed namespace when building the final route, but does not pass the sanitized form to the server.

<?php
$full_route = '/' . trim( $namespace, '/' ) . '/' . trim( $route, '/' );
rest_get_server()->register_route( $namespace, $full_route, $args, $override );

I think this would have resulted in broken behavior previously, but would have been much more subtle. I believe your route wouldn't properly appear in the index.

We could fix this in core by doing something like this so the namespace parameter is forced to be valid. cc: @kadamwhite

<?php
$namespace = trim( $namespace, '/' );
$full_route = '/' . $namespace . '/' . trim( $route, '/' );
rest_get_server()->register_route( $namespace, $full_route, $args, $override );

For now, my recommendation would be to drop the preceding slash.

[afercia]

Seems to me the namespace pattern is well documented, right?
​https://developer.wordpress.org/rest-api/extending-the-rest-api/adding-custom-endpoints/#namespacing

If the leading slash is confirmed as the cause of the reported issue, I'd say this is not a bug and the /wp/v2 pattern worked in WordPress 5.3.2 only "by accident".

[skarabeq]

@afercia, I do not agree with you. I have use this namespace /wp/v2 more than 2 years. So this isn't "accident".
@TimothyBlynJacobs about this code which is in your core:

<?php
$full_route = '/' . trim( $namespace, '/' ) . '/' . trim( $route, '/' );

And your suggested code

<?php
$namespace = trim( $namespace, '/' );
$full_route = '/' . $namespace . '/' . trim( $route, '/' );

The output of the $full_route will be same.

[TimothyBlynJacobs]

The output of the $full_route will be same.

@skarabeq the output of the full route is the same, but notice that the namespace passed will now be forced to have correct formatting.

I do not agree with you. I have use this namespace /wp/v2 more than 2 years. So this isn't "accident".

I do not believe that prefixing the namespace with a slash is expected behavior, and I agree with @afercia that it is documented fairly clearly.

The existing routing worked because of the coercion that was applied. But this was previously broken in other ways. Any code that relied upon a valid namespace value would be broken. For example, you can see this when doing a request for /wp-json/wp/v2, the nav endpoint won't be listed.

That doesn't mean we won't also add that coercion to the namespace parameter as I described earlier. But I think your best course of action would be to correct your route definition while we wait for 5.4.1.

[skarabeq]

@TimothyBlynJacobs , thank you for your replies. Do you know when will be introduced WP 5.4.1?

[TimothyBlynJacobs]

I do not. 5.4.1 may be discussed at the dev chat today at 21:00 UTC

[SergeyBiryukov]

Moving for 5.4.1 consideration.

[skarabeq]

@TimothyBlynJacobs, one more thing about this problem. If there is the namespace which is not named with word wp then I'm able to use the namespace with a slash at the beginning of the namespace, but if there is the namespace - wp and it starts at the beginning with the slash, then I have the described problem. So I think that your solution is just workaround.

[TimothyBlynJacobs]

It is not a workaround, a namespace with a prefixed slash is not valid. It is able to serve the routes when you register your own namespace with a prefixed slash because all the routes are belonging to the same group. If you had registered both /my-ns and my-ns as a namespace you'd have the same issue.

And, if you register that route, the index will not work for you.

<?php
add_action('rest_api_init', function() {
        register_rest_route(
                '/lp/v2',
                '/nav/',
                array(
                        'methods'  => 'GET',
                        'callback' => static function () {
                                return new WP_REST_Response( null, 204 );
                        },
                )
        );
});

curl https://trunk.test/wp-json/lp/v2/
{"code":"rest_no_route","message":"No route was found matching the URL and request method","data":{"status":404}}

[skarabeq]

@TimothyBlynJacobs, Ok, I agree that in the world of the WordPress the route with slash is not valid.. But I can't understand that because the namespace of route with slash and the namespace of the route without slash should be same. If someone make two routes with the same namespace and endpoint, then he is made something wrongly. You can get best practice in the one of the best PHP frameworks - Laravel - Routing - ​https://laravel.com/docs/7.x/routing.
One of the best pattern of the grouping routes is like this:

<?php
Route::prefix('admin')->group(function () {
    Route::get('navigation', function () {
        // Matches The "/admin/navigation" URL
        return AdminModelNavigation::getAll();
    });
    Route::get('users', function () {
        // Matches The "/admin/users" URL
        return AdminModelUsers::getAll();
    });
});

But, WordPress is far, far away from this path. So do the best, and if you fix the problem that I meet I will be very happy :)

Kind regards,

[TimothyBlynJacobs]

But I can't understand that because the namespace of route with slash and the namespace of the route without slash should be same.

Yes, as I mentioned earlier, the route is coerced to be the same. But that coercion did not apply to the actual namespace value. The change we'll make is to apply that coercion to the namespace as well.

This will be fixed in 5.4.1, but at the moment that doesn't look at least a few weeks out.

[whyisjake]

@TimothyBlynJacobs I am going to bump from 5.4.1 since we don't have a patch/testing yet.

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

[TimothyBlynJacobs]

I've uploaded a PR that issues a _doing_it_wrong notice and cleans the namespace to be valid.

[whyisjake]

In 47842:

REST API: Ensure proper namespacing when registering routes.

The PR will corerce routes that have a leading slash and throwing a _doing_it_wrong notice while ensuring a proper namespace.

Fixes #49749.
Props TimothyBlynJacobs, skarabeq, afercia.

[whyisjake]

In 47843:

REST API: Ensure proper namespacing when registering routes.
The PR will corerce routes that have a leading slash and throwing a _doing_it_wrong notice while ensuring a proper namespace.

This brings the changes from [47842] to the 5.4 branch.

Fixes #49749.
Props TimothyBlynJacobs, skarabeq, afercia, skithund.