Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

404 on REST API with "lang" query #720

Closed
herrvigg opened this issue Aug 13, 2019 · 3 comments
Closed

404 on REST API with "lang" query #720

herrvigg opened this issue Aug 13, 2019 · 3 comments
Labels
bug Something isn't working, reproducible core Core functionalities, including the admin section

Comments

@herrvigg
Copy link
Collaborator

If a lang=xx query var is sent to a REST endpoint, a redirection is triggered in qTranslate core, ending most likely in a page not found error (404).

This is quite a specific failure case for the lang query var (both with GET and POST requests), when the current language is not the default one & when the query mode is not QTX_URL_QUERY (default is QTX_URL_PATH).

The REST API should never redirect!

@herrvigg herrvigg added the bug Something isn't working, reproducible label Aug 13, 2019
@herrvigg herrvigg added the core Core functionalities, including the admin section label Oct 13, 2019
@herrvigg
Copy link
Collaborator Author

herrvigg commented Oct 20, 2019

I pushed the investigation because this is related to Gutenberg and #744 . The problem comes from the ambiguity of forcing a language switch through the query var lang, while not being in URL query mode (path mode but it should be the same with domains).

It is quite easy to reproduce the bug. Set URL mode to QTX_URL_PATH, with 2 languages e.g. en and fr. Set hide default language with default = en.

Standard REST calls without forcing the language switch:

  • http://localhost:8000/wp-json/wp/v2/posts/24056 OK en
  • http://localhost:8000/en/wp-json/wp/v2/posts/24056 OK en ( with correct permalinks)
  • http://localhost:8000/fr/wp-json/wp/v2/posts/24056 OK fr

Now if we try to force a language switch on REST (ugly but it can happen):

  • http://localhost:8000/wp-json/wp/v2/posts/24056?lang=fr SWITCH en -> fr
  • http://localhost:8000/en/wp-json/wp/v2/posts/24056?lang=fr SWITCH en -> fr
  • http://localhost:8000/fr/wp-json/wp/v2/posts/24056?lang=en FAIL 404

The problem on the last case is that the home_url will return http://localhost:8000/ while being on http://localhost:8000/fr/ so the REST url is not resolved by WP... unless you redirect. And we don't allow this here.

But this feature of forcing language switch should only be for a regular non-REST URL in the browser. Imo it is wrong to expect the REST call to switch language while being in PATH mode. So it would be better to disable it, i.e. ignoring the lang query var for any REST call, unless we are in query mode. This makes qtranxf_parse_language_info more complicated and forces the web administrator to use correct REST links, but i think it is the best solution.

@herrvigg
Copy link
Collaborator Author

As a side-effect of these changes, the cookie may now be read as the fallback language for REST for the front case. In one sense, that's a bit less RESTful because there is a state hold out of the request.

The rationale behind this is when you have a hidden language, your REST calls from within a page should rather use the current language being set in the browser, rather than the default language. In any cases, the cookie should only be seen as a the current user preference to override the default language.

Ideally you should always have the language explicitly defined in the query, but that's not going to be the case for many of the REST URLs set in the page, so better have the REST calls more consistent with the current state of the browser.

If you use REST calls outside the browser, the default language should be used as before.

There are some potential evolutions for a better customization of the REST behavior in the future but this would first require a deep refactoring of the code. Currently it's been already very difficult to come up with these changes.

@herrvigg
Copy link
Collaborator Author

Released in 3.7.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working, reproducible core Core functionalities, including the admin section
Projects
None yet
Development

No branches or pull requests

1 participant