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.

Sign up for GitHub

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

Jump to bottom

Redirections cancelled by qtranxf_can_redirect should not assert #1326

Closed
herrvigg opened this issue Apr 17, 2023 · 1 comment
Closed

Redirections cancelled by qtranxf_can_redirect should not assert #1326

herrvigg opened this issue Apr 17, 2023 · 1 comment
Labels
bug Something isn't working, reproducible core Core functionalities, including the admin section severity: critical Critical functionality

Comments

@herrvigg
Copy link
Collaborator

There's an inconsistency between the qtranxf_can_redirect check and the assert that is triggered if the request can't be redirected.

The idea of assert was to notify to the user wrong requests with undefined language. However that doesn't hold well for example with AJAX requests coming from third-party plugins (not detected as admin requests), CLI commands, ... Ideally the language should be properly set, but it is impossible to assume that all plugins use the WordPress URL functions that are hooked by qTranslate such as home_url(). There might be several issues connected to that problem.

Most of the requests should still work using the default language or first in the list (to be clarified). For some cases example update requests writing in DB it could be problematic to use the wrong language but that would be a developer bug due to a wrong URL usage, not a bug from qTranslate.

For now the best is to simply skip the assert. Even a warning would be problematic for example with AJAX requests.
In the future we may want to notify these cases differently for the admin for example with separate logs that won't impact the HTTP functionalities.
@herrvigg herrvigg added bug Something isn't working, reproducible severity: critical Critical functionality labels Apr 17, 2023
This was referenced Apr 17, 2023
Closed
Closed
Closed
Closed
Closed
herrvigg added a commit that referenced this issue Apr 17, 2023
@herrvigg
Fix cancelled redirections should not assert (#1326)
8c9e672 
Remove the assert that is inconsistent with cancelling the redirect.
Add a finer notice in the URL info but don't output any message
that could break the HTTP functionalities.
Fix some typos in redirect comments.
@herrvigg herrvigg mentioned this issue Apr 17, 2023
Closed
herrvigg added a commit that referenced this issue Apr 17, 2023
@herrvigg
Fix URL info notice on cancelled redirect (#1326)
b2b7c16
herrvigg added a commit that referenced this issue Apr 18, 2023
@herrvigg
Refactor with qtranxf_check_url_maybe_redirect (#1326)
73b7d90
herrvigg added a commit that referenced this issue Apr 19, 2023
@herrvigg
Fix wrong assert with cancelled redirections (#1326)
bc77eac 
Remove the assert that is inconsistent with a redirect not allowed
for example with Ajax requests, CLI and cron commands.

Refactor the logic with `qtranxf_check_url_maybe_redirect`.
Don't output any message that could break the HTTP functionalities.
@herrvigg herrvigg added the core Core functionalities, including the admin section label Apr 19, 2023
@herrvigg
Copy link
Collaborator Author

Fix released in 3.14.2.
Should also fix #766, #986, #1068, #1096, #1266, #1284.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working, reproducible core Core functionalities, including the admin section severity: critical Critical functionality
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@herrvigg