Redirects on REST API

[herrvigg]

hello,
some tickets have been raised about redirects on REST API calls. Here are the duplicated tickets in this repo: #575, #528, #489, #427. These tickets were created in the original qTranslate-X (not maintained) by @mikaelz @venkatraj @ephor @esiao. Please could some of you check this with qTranslate-XT?

Currently qTranslate-XT redirects some REST calls involving the default language. It's possible to disable the redirects but i'm not completely sure about the right solution. There are quite many combinations to test and maybe it depends on how the API is used. Maybe some issues with Jetpack that were fixed? I don't know.

Anyway i tried the given PRs but it actually went worse :-/
Let's take an example: local site with english and french language, default is french. Here i query a post with a GET. I haven't tested with POST requests yet.

Pre-Path Mode (/lang/...) + hide default

With the original code there is a redirect for the default language.

localhost/wp-json/wp/v2/posts/13498 -> french content OK (default)
localhost/fr/wp-json/wp/v2/posts/13498 -> redirect to default url above, french OK.
localhost/en/wp-json/wp/v2/posts/13498 -> english content OK

It seems the 2d point is the problem mentioned in the tickets but for this test it's fine so i don't understand the issue yet.

Now if i disable the redirect:

localhost/wp-json/wp/v2/posts/13498 -> french content OK (default)
localhost/fr/wp-json/wp/v2/posts/13498 -> error! Page not found 404 :(
localhost/en/wp-json/wp/v2/posts/13498 -> english content OK

Here the disable introduced a regression :-/

Pre-Path Mode + no hide default

localhost/wp-json/wp/v2/posts/13498 redirects to localhost/fr/wp-json/wp/v2/posts/13498

Similar to above but the other way around. Any idea when this redirect would be a problem?
If i disable the redirect this time it works fine, no 404.

Query Mode (?lang=xx)

This mode shouldn't be a problem since there are no redirects. I tried only with hide default language and all worked fine:
localhost/wp-json/wp/v2/posts/13498?lang=en -> english content OK
localhost/wp-json/wp/v2/posts/13498?lang=fr -> french content OK (default)
localhost/wp-json/wp/v2/posts/13498 -> no redirect, french content OK.

[herrvigg]

Note there is a filter in qTranslate called qtranslate_language_detect_redirect. This is the best way to avoid redirections without patching this plugin code. However it's not feasible in a theme because the hook would be set too late. It has to be done before plugins_loaded so it's limited to plugins having a special interaction with qTranslate.

Now i also found this from Jetpack. They actually use this hook! This fix was done in Jetpack 5.3 released on September 05, 2017. So i think some of the issues could be solved with this already, especially #489 and #528.

From jetpack/3rd-party/qtranslate-x.php:

/**
 * Prevent qTranslate X from redirecting REST calls.
 *
 * @since 5.3
 *
 * @param string $url_lang Language URL to redirect to.
 * @param string $url_orig Original URL.
 * @param array $url_info  Pieces of original URL.
 *
 * @return bool
 */
function jetpack_no_qtranslate_rest_url_redirect( $url_lang, $url_orig, $url_info ) {
	if ( false !== strpos( $url_info['wp-path'], 'wp-json/jetpack' ) ) {
		return false;
	}
	return $url_lang;
}
add_filter( 'qtranslate_language_detect_redirect', 'jetpack_no_qtranslate_rest_url_redirect', 10, 3 );

[herrvigg]

Mmm ok i start to better see the problem while doing some experiments for Gutenberg. We should definitely avoid any redirection on the REST calls. Let's try to find a robust solution.

[herrvigg]

This becomes an issue to fix in priority. I'll come back to it soon.

[herrvigg]

These redirections are quite a tricky problem. If you use the "hide default" option (which i use for instance) there is actually a redirection if your front cookie does not match the default language. This should not be done on an API REST call!

I think i found a better solution to avoid the redirections but i have now an issue with one case when using explicitly the default language. The REST request cannot be parsed in that case, leading to the 404 error. But now i understand why it fails. The main trick with qTranslate is that it overrides the home_url by adding the language code so that all the standard rewrites can work. However this doesn't work when forcing the default language.

Examples with fr as default language:

Expected behavior with hide default, no redirections allowed:
localhost/wp-json/wp/v2/posts/13498 -> should always return fr content, independently of any cookie
localhost/fr/wp-json/wp/v2/posts/13498 -> ideally should return fr but doesn't work because home_url() = "localhost" and not "localhost/fr" due to hidden default
localhost/en/wp-json/wp/v2/posts/13498 -> should always return en content, it works because home_url() = "localhost/en"

It could be possible to handle home_url specifically for REST request but i haven't found yet a nice way to do it without assuming how the REST request is processed before hand.

[herrvigg]

So i think i found a reasonable solution to prevent these nasty API REST redirects:

• REST calls should never redirect! Added a check with qtranxf_is_rest_request_expected based on rest_get_url_prefix.
• REST calls should be fully deterministic! Updated qtranxf_detect_language : REST calls should ignore any front/admin cookie and any other weird language detection.
• new init action qtranxf_rest_api_register_rewrites for specific rewrite rules to handle the case i described earlier, when the default language is hidden in the options, but asked in the request. Note to make it work you have to flush your rewrite rules by saving the permalink structures from the admin page!

I'm quite confident it should work so i'm very tempted to push this to master, also because i want most of people to validate it, but i'll wait a few days giving you the possibility to try it first in this fix branch:

https://github.com/qtranslate/qtranslate-xt/tree/fix/rest_no_redirect

What's behind?

qtranxf_is_rest_request_expected
This is only a predective search to detect the REST calls with a kind of simple heuristic from the REQUEST_URI. Using rest_get_url_prefix is much better than using the hard-coded wp-json string. But the REST API protocol is more complicated than just checking for a prefix, it is resolved much later in parse_request (class-wp.php) with the query rewrites. So we might wrongly prevent redirections if this prefix string is somewhere else in the query. However it's a bit difficult to put more constraints on this and it should work in 99.99% of the cases, unless you have a very strange config or have some weird URLs (who would use wp-json for other usage than for he API REST ?!). This is more of a practical solution and this function might be improved in the future if really needed.

qtranxf_rest_api_register_rewrites
More details are given in the function description. But this functions adds some rewrites inspired by the official API REST (rest_api_register_rewrites in rest_api.php) to handle the special case with the default language. All the other cases should be handled naturally through the home_url overridden by qTranslate-XT (unchanged, as for -X). More about the URL modes:

• this fix is intended for the QTX_URL_PATH mode only (aka "pre-path mode").
• the QTX_URL_QUERY mode should not be concerned
• what i'm less sure about is how to handle QTX_URL_DOMAIN and QTX_URL_DOMAINS. I guess it should work without needing a specific case but i'm not really able to test this, sorry. So if someone use this mode it would be great if you could validate it.

Also, we might miss very specific REST calls using index.php/wp_json but also here i'm not sure who would use this kind of horrible URLs :P

[herrvigg]

I created a PR so please try it and comment it there. I lock this ticket for now.

[herrvigg]

I have now merged this PR into master so more people can validate this!

[herrvigg]

Fix released in qTranslate-XT 3.5.3.