Page MenuHomePhabricator

Core REST API page creation endpoint returns invalid redirect
Closed, ResolvedPublic

Description

Expected behavior

The page creation endpoint returns a Location header with valid API routes to fetch the corresponding page.

Observed behavior

The Location header URLs misses the path prefix indication the API version, making it invalid.

See also

T252566: Core REST API page history endpoint returns invalid links

Event Timeline

See T252566#6149529 for an explanation of the problem.

The solution discussed there leaves the problem of one handler discovering another handler's path (or full URL). We can know the other endpoint's "logical" path (without the prefix), and we can know its class. But how do we find the full path, including the prefix?

The path of another endpoint would have to be looked up in the Router. But based on what? We could but a logical "name" for each route into the route definition JSON. Then the Router could look up the path based on that. I don't really see any other way...

To catch this, the integration test in tests/api-testing/REST/Creation.js needs to have

assert.match( normalizedLocation, new RegExp( `^https?://.*/page/${encodedTitle}$` ) );

replaced with

assert.match( normalizedLocation, new RegExp( `^https?://.*/v1/page/${encodedTitle}$` ) );
daniel triaged this task as High priority.May 19 2020, 7:23 PM

If we aliases for the routes, we could have a method like Router::getRouteUrl( $routeAlias, $pathParams = [], $queryParams = [] ).

Talking to Tim, I just realized that if we want to support multiple versions for the same route, the version prefix for the target route can't be automatic. It has to be hard coded. So all these thoughts about a nice way to discover the fully path are pointless, we just have to hard code the version prefix when constructing the url for a route.

Change 598425 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] CreationHandler: fix redirect URL

https://gerrit.wikimedia.org/r/598425

Change 598425 merged by jenkins-bot:
[mediawiki/core@master] CreationHandler: fix redirect URL

https://gerrit.wikimedia.org/r/598425

Here's the request I used to try and test this:

$ curl -v -X POST https://meta.wikimedia.org/w/rest.php/v1/page -H "Content-Type: appication/json" -H "Authorization: Bearer $TOKEN" --data '{"source": "Hello, world!", "title": "User:APaskulin (WMF)/Sandbox/APITEST2", "comment": "Creating a test page with the REST API"}'

I got a successful response with the header:

location: https://meta.wikimedia.org/w/rest.php/v1/page/User%3AAPaskulin+%28WMF%29%2FSandbox%2FAPITEST2

However, trying to call the URL in the location header, I get a 404 title not found error. The page has been created at User:APaskulin_(WMF)/Sandbox/APITEST2, but the location link includes a +, making it invalid.

@apaskulin Interesting! I tested with a page without a space in it, and it worked OK.

Since this was originally about the route prefix (/v1/ or /coredev/v0), maybe we should start a new ticket for titles with spaces in them?

My issue has been opened as a separate task, T258606. (Thanks, Evan!) Moving this out of the Documentation column since no doc updates are required.

I ran into a similar issue when working on the user contributions api. The issue is that spaces can and should be encoded as plus ("+") in query strings, but not in the path. In php, this amounts to the difference between calling urlencode and rawurlencode. I thought we fix this along the way, I'll have a look.

Since this is a bug fix, I don't see any documentation needs.