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

Problems with cached 302 language redirects with W3 Total Cache browser caching #306

Closed
lklapa opened this Issue Nov 6, 2015 · 21 comments

Comments

Projects
None yet
3 participants
@lklapa
Copy link

lklapa commented Nov 6, 2015

I have faced a bug that prevents qTranslate-X from switching languages properly. I'm using WordPress 4.3.1 and qTranslate-X 3.4.6.4. My setup has two languages: pl (default) and en.

Steps to reproduce:

  1. Visit http://website.domain/, and detected language is PL (my browser's locale) - all fine.
  2. Switch to EN, and in network console log I can see a 301 redirect to /en/ - the website is now in EN as expected - correct.
  3. Visit once again http://website.domain/, and resulting page comes in EN - correct.
  4. Visit once again http://website.domain/, but this time the 302 redirect is being served from browser's cache.
  5. Now, when you try to switch the website back to the default language (PL), the network log shows a request to /pl, then the server responds with a redirect to / (as I'm using an option to hide the default language), and now browser cache interferes saying according to the result cached in step 4, that / should be redirected to /en.

At this stage it is impossible to switch back to the default PL language, as all requests looking for GET / are from now on treated according to browser cache, so GET / = [cached 302] = GET /en.

Even without the option to hide the default language, visiting http://website.domain/ still uses browser's cache, so I'm ending up in a redirection leading me towards /en until I clear the cache.

Such behavior is not observed if I disable web browser cache option in W3 Total Cache plugin.

The solution:
inside qtranxf_init_language function (line 80), there is a call to wp_redirect().

Changing it from:

wp_redirect($target);

to:

wp_redirect($target, 303);

solves the issue for me, as 303 redirects according to the standard are not to be cached in the browser.

Any chance for changing this redirect in the next qTranslate-X release?

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented Nov 6, 2015

Thanks a lot for the report of this unusual issue. Does W3 Total Cache work in all other respects? I would assume that there are many other problems. Generally every caching plugin tries to be smart and caches some queries, which in fact run differently for pages with different languages, but they have no way to take that into account. It caused me to give up using caching for now.

But things are improving every day, may be now W3 Total Cache is usable with qtx. Please, let me know if you find more issues. How complex is your site? Do you use Woocommerce or something like that?

About redirect. 303 is redirect with method GET, which will mostly work, but would not it be safer to use 307 then? What does W3 Total Cache do exactly to make browser to cache redirects?

@johnclause

This comment has been minimized.

@lklapa

This comment has been minimized.

Copy link
Author

lklapa commented Nov 6, 2015

W3 Total Cache seems to work fine - at least I didn't notice any unexpected behavior.

The site is not really complex - it has a few static pages, and a number of posts in various categories, but generally most of the content is WordPress based (apart from a few additions for sliders/Google map and so on). No Woocommerce here.

Well, if 307 is a safer choice, it might be used as well. I can perform the same testing using 307 instead of 303. Regarding the question about the cause - I can revert to simple wp_redirect($target) and note when the caching takes place exactly (with which request). Generally speaking I noticed that with browser cache function enabled in W3 Total Cache, additional headers are being sent out with the requests while changing languages and also serving all content, and I believe these are causing the browser to cache the redirect requests.

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented Nov 6, 2015

Yes, please test 307. Thanks a lot. Maybe we should make that code to be an option?

@lklapa

This comment has been minimized.

Copy link
Author

lklapa commented Nov 6, 2015

An option would be a great move IMO. I will get back to you after I compare those response headers with and without browser cache function in W3 Total Cache.

@lklapa

This comment has been minimized.

Copy link
Author

lklapa commented Nov 7, 2015

Ok, so I did the testing. Whenever browser cache is enabled, response headers contain additionally (comparing to this option disabled) these fields while doing the redirect:

Cache-Control: max-age=10000
Expires: Sat, 07 Nov 2015 01:18:45 GMT
Vary: User-Agent

I have got a huge file with all of these headers, but I think that listing the differences is enough at this stage.

I have also made a few interesting observations:

  • Those 5 steps I described to replicate this cached redirect behavior can only be observed while I am logged in to WordPress, so I am browsing the page having this admin bar displayed at the top of the screen. Without being logged in to WordPress the page seems to work fine up to a certain point (see below).
  • I have also came across similar behavior when the page stucks on switching languages without being logged in to WordPress, but unfortunately I am still working on it to figure out the steps to reproduce it. What I noticed is that when I was in default (PL) version on the website, the link to a page from website menu showed up correctly (say, /my-page-name-in-pl), but after clicking it the browser was redirected to /en/my-page-name-in-en each time until I cleaned up browser cache.
  • During the testing with wp_redirect($target, 307); the redirect request in step 5 was served from a web browser cache. With wp_redirect($target, 303); such behavior could not be observed.
  • There is absolutely no difference (of course apart from Date and Expires fields) between response headers while redirecting with 303 or 307, but only 307 gets cached.

Currently I am using a simple wp_redirect($target);, but still can not figure out when the page starts to mix up with cached redirects. Hopefully I will get more information soon. The whole problem with cached redirects is that I do not know yet when they actually get cached. Some visitors complained about being unable to either switch the language after an unknown path of clicks, or the website started to produce links that pointed to a non-existing location because of redirects (for example /en/my-page-name-in-pl).

@lemartinet

This comment has been minimized.

Copy link

lemartinet commented Apr 25, 2016

Hi,
I add a problem similar to the one described by @lklapa (not being able to switch from one language to the other because of a cache redirect), and the solution of a 303 redirect seems to be working! It would be great to have that integrated in the plugin!
Anyway, thanks everybody for the great work on this plugin.
Best
Manu

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented Apr 26, 2016

WordPress uses 302 as a default choice for their wp_redirect and they use it a lot. I did a search and they never use anything, but 301 and 302, and 301 is only in the places to redirect obsolete code. How does that work for them?

Having this as an advanced option is ok with me if somebody implements it and submit a pull request ;)

Are you both sure that there is no a simpler solution? Maybe W3 Total cache has some options to deal with it? Probably not though ...

Let us hope that somebody will take an initiative to implement an option.

@lklapa

This comment has been minimized.

Copy link
Author

lklapa commented Apr 27, 2016

Hi,

For us the fix with 303 redirect was mostly working. However, on rare occasions visitors ended up being redirected from the correct url to a non-existing one, resulting in 404. I agree that if WordPress uses 302 as a default choice, then the problem might be somewhere else.

I did some more digging into this, and it turned out that the mechanism responsible for language detection might be involved. I've gathered some print_r outputs from the plugin work and my observations lead to the conclusion that if there is a conflict between the cookie and url language, the website might throw 404.

Consider the following:
The part of the original request: [REQUEST_URI] => /categoryPL/articlePL/

Dump from qtranxf_init_language: detected: url_info:

    [cookie_enabled] => 1
    [scheme] => https
    [host] => website.domain
    [path] => /categoryPL/articlePL/
    [original_url] => /categoryPL/articlePL/
    [path-base] =>
    [path-base-length] => 0
    [doing_front_end] => 1
    [wp-path] => /categoryPL/articlePL/
    [lang_cookie_front] => en
    [doredirect] => language needs to be shown in url
    [language] => en
    [set_cookie] => 1

Resulting debug message:

qtranxf_init_language: doredirect to en
urlorg:https://website.domain/categoryPL/articlePL/
target:https://website.domain/en/categoryPL/articlePL/

The worst thing with trying to figure out this behaviour is that really all works just fine in most of the cases :) You can switch languages either through a button, or simply by adding /en/ in the URL and all works as expected.

At times however, even though in the original URL there is no /en/ (just as a reminder: my website has a default pl language that is hidden in the URL part), the plugin enforces the redirection from /categoryPL/articlePL/ to /en/categoryPL/articlePL/, which unfortunately results in 404. If the redirect would have been built including the translation of category and article (giving /en/categoryEN/articleEN/, then I assume all would be fine.

Temporarily we are using an ugly hack, that sets the $url_info['language'] to en if the URL has /en/ part in it, and pl otherwise. I know this is not a solution at all, but this one simply work for us. I wish I had more time to be able to debug it further, and I really wish this error was easier to reproduce ;) @johnclause - any particular hints where could I start? It might be that the whole story begins with W3TC or Qtranslate Slug. @lemartinet - could you please specify which (if any) plugins (translation- and cache-related) are you using on your site?

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented Apr 27, 2016

QTX does not manipulate slugs. In QTX, redirection /categoryPL/articlePL/ to /en/categoryPL/articlePL is the only what it does, and, as you described, it does it correctly. You have to use QTranslate-Slug to achieve redirection /categoryPL/articlePL/ to /en/categoryEN/articleEN? QTranslate-Slug mostly works fine as people report.

I never realised that you use QTranslate-Slug and you should definitely discuss this issue with them and not here.

@lemartinet: do you also use QTranslate-Slug?

@lklapa

This comment has been minimized.

Copy link
Author

lklapa commented Apr 27, 2016

@johnclause you are right, sorry for that. I really thought I included my full set-up in the first message.
I am definitely going to see how QTranslate-Slug is being invoked and what influence it has on the redirection.
It would be interesting though to hear more details on the set-up from @lemartinet.

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented Apr 27, 2016

Let the authors know about this topic: https://github.com/not-only-code/qtranslate-slug

@lemartinet

This comment has been minimized.

Copy link

lemartinet commented Apr 27, 2016

Hi
Sorry for the slow answer, a bit busy. I'm not using Qtranslate-slug or
w3tc. For cache I'm using a nginx+redis solution offered through
easyengine, which provides page cache, browser cache and object cache.
Let me know if I'm not being precise enough or if I can help more.
Manu
On Apr 27, 2016 3:58 PM, "johnclause" notifications@github.com wrote:

Let the authors know about this topic:
https://github.com/not-only-code/qtranslate-slug


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#306 (comment)

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented May 5, 2016

Hello guys, is that possible to include a directive in the header to disable all caching for the current request? I guess, this would solve all the problems in a simplest and safest way?

What would be the syntax of such directive?

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented May 6, 2016

Seems like this works:

                wp_redirect($target);
                header("cache-control: must-revalidate, max-age=0, no-cache");

Could you all try it, please?

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented May 6, 2016

Edit: I switched two lines around in the previous message so that "header" goes after "wp_redirect". I was testing on the server where caching is induced by .htaccess file and then the order does not matter. But in case of caching plugin, it may employ filters inside wp_redirect to add its own "cache-control" directive, which we need to overwrite.

Alternatively, it maybe even better to do:

                header("cache-control: must-revalidate, max-age=0, no-cache");
                header('Location: '.$target, true, 302);

to disable filters inside wp_redirect. Please, let me know how and if it works for you.

@lemartinet

This comment has been minimized.

Copy link

lemartinet commented May 6, 2016

Thanks for the idea, I'll try to play with it this WE and let you know the
result

Manu

On Thu, May 5, 2016 at 11:48 PM, johnclause notifications@github.com
wrote:

Edit: I switched two lines around in the previous message so that "header"
goes after "wp_redirect". I was testing on the server where caching is
induced by .htaccess file and then the order does not matter. But in case
of caching plugin, it may employ filters inside wp_redirect to add its
own "cache-control" directive, which we need to overwrite.

Alternatively, it maybe even better to do:

            header("cache-control: must-revalidate, max-age=0, no-cache");
            header('Location: '.$target, true, 302);

to disable filters inside wp_redirect. Please, let me know how and if it
works for you.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#306 (comment)

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented May 7, 2016

I updated https://github.com/qTranslate-Team/qtranslate-x/archive/stable.zip with a change. WP even have a function nocache_headers() for such a case ;)

I am closing this issue, assuming it works now, please test, we can still write into closed issue and we can re-open it if needed. Thank you all for your help.

@johnclause johnclause closed this May 7, 2016

@lemartinet

This comment has been minimized.

Copy link

lemartinet commented Jun 16, 2016

Hi,
It seems that I still get a related issue, and several readers of our blog have notified us. The way it happens is the following:

I'm assuming it's somehow related to the same caching issue. Do you have an idea how to test and fix that?

Thanks!

@johnclause

This comment has been minimized.

Copy link
Member

johnclause commented Jun 16, 2016

I could not reproduce the problem. I tried in a few browsers and on my phone. Maybe they are your old customers who still have that permanent redirection in their browsers? I am not sure how long it takes to expire?

@lemartinet

This comment has been minimized.

Copy link

lemartinet commented Jun 16, 2016

Thanks for your answer. I'll keep you posted if I have more info about this
issue.

On Thu, Jun 16, 2016 at 3:12 PM, johnclause notifications@github.com
wrote:

I could not reproduce the problem. I tried in a few browsers and on my
phone. Maybe they are your old customers who still have that permanent
redirection in their browsers? I am not sure how long it takes to expire?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#306 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AHMNEke0nO3TTym4vvKUBckIMd6hMe39ks5qMaADgaJpZM4GdopH
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.