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

Duplicate Location headers after 302 redirects #1099

Closed
HansVanEijsden opened this Issue Jan 22, 2016 · 21 comments

Comments

Projects
None yet
5 participants
@HansVanEijsden
Copy link

HansVanEijsden commented Jan 22, 2016

After doing a 302 redirect, browsers like Safari Desktop and Safari Mobile can't display the page. Probably because of duplicate Location headers.
The problem also occurs after trying to login to Wordpress, RoundCube webmail or Joomla (all of those give redirects after a successful login).

To replicate:

$ curl -I https://www.fundays.nl
HTTP/1.1 302 Moved Temporarily
Server: nginx
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
Keep-Alive: timeout=300
Set-Cookie: wb_4647-sid=0r1guonnvfl6sa861oh3noc055; path=/; HttpOnly
Location: https://www.fundays.nl/pages/fundaysfestival/fundays-live.php
Access-Control-Allow-Origin: *
Strict-Transport-Security: max-age=31536000;
Location: https://www.fundays.nl/pages/fundaysfestival/fundays-live.php
Date: Fri, 22 Jan 2016 15:46:48 GMT
X-Page-Speed: 1.10.33.2-7600
Cache-Control: max-age=0, no-cache
$ curl -I https://www.fundays.nl?ModPagespeed=Off
HTTP/1.1 302 Moved Temporarily
Server: nginx
Date: Fri, 22 Jan 2016 15:45:15 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
Keep-Alive: timeout=300
Set-Cookie: wb_4647-sid=a8r6cdu8bl1mvnouch3v9sbui3; path=/; HttpOnly
Location: https://www.fundays.nl/pages/fundaysfestival/fundays-live.php
Access-Control-Allow-Origin: *
Strict-Transport-Security: max-age=31536000;

Tested with ngx_pagespeed 1.10.33.2-7600 on Nginx 1.9.9.
Basic Nginx config (without Pagespeed parts): http://pastebin.com/FtDdGTeX

If you need the Pagespeed config parts, just let me know. The problem also occurs with all the Pagespeed filters disabled (but Pagespeed loaded of course).

More info: https://groups.google.com/forum/#!topic/pagespeed-dev/0JG3fqFUaWw

@crowell crowell added the bug label Jan 22, 2016

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Jan 22, 2016

Consider 1f02f36 and its re-application later on for 1.10 01e458c

They look different, which may need closer inspection

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Jan 22, 2016

No this seems to be a dead-end, I can't spot a problem with 1f02f36 vs 01e458c

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Jan 22, 2016

Oh wait, but 01e458c was added fairly recently and is not available in 1.10.33.2

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 22, 2016

Otto noticed that 059dd20 didn't make it into 1.10 because it was merged to the wrong place. That's probably it.

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 22, 2016

Hoping for it. Thanks!

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 22, 2016

@HansVanEijsden Would you be able to try cherry-picking 059dd20 to verify that this fixes it?

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 22, 2016

@jeffkaufman I would love to, but I can't find the instructions on how to install from source.
https://developers.google.com/speed/pagespeed/module/build_ngx_pagespeed_from_source only mentions downloading the zip file, unfortunately..

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 22, 2016

Sorry about that! Here's up to date instructions for building from git:

Where it says:

wget https://github.com/pagespeed/ngx_pagespeed/archive/release-${NPS_VERSION}-beta.zip \
     -O release-${NPS_VERSION}-beta.zip
unzip release-${NPS_VERSION}-beta.zip

Instead do:

git clone git@github.com:pagespeed/ngx_pagespeed.git
mv release-${NPS_VERSION}-beta release-${NPS_VERSION}-beta-old # in case you have one already
mv ngx_pagespeed release-${NPS_VERSION}-beta

Then after you do cd ngx_pagespeed-release-${NPS_VERSION}-beta/, run

git checkout release release-${NPS_VERSION}-beta

Now's the place for the cherry-pick:

git cherry-pick 059dd20

And then continue on with the rest of the build, starting with wget https://dl.google.com/dl/page-speed/psol/${NPS_VERSION}.tar.gz

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 22, 2016

No need to say Sorry Jeff. I never asked about it before. 😉

Unfortunately I encounter some errors. I tried to correct them as far as possible, but I'm stuck at doing the right git checkout. Here are the steps I've done (system language is in Dutch, nl_NL, sorry):

hans@vps:$ cd
hans@vps:
$ NPS_VERSION=1.10.33.2
hans@vps:$ git clone git@github.com:pagespeed/ngx_pagespeed.git
Cloning into 'ngx_pagespeed'...
Permission denied (publickey).
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
hans@vps:
$ git clone https://github.com/pagespeed/ngx_pagespeed.git
Cloning into 'ngx_pagespeed'...
remote: Counting objects: 8552, done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 8552 (delta 0), reused 0 (delta 0), pack-reused 8547
Receiving objects: 100% (8552/8552), 41.63 MiB | 15.97 MiB/s, done.
Resolving deltas: 100% (4139/4139), done.
Checking connectivity... done.
hans@vps:$ mv ngx_pagespeed release-${NPS_VERSION}-beta
hans@vps:
$ cd ngx_pagespeed-release-${NPS_VERSION}-beta/
-bash: cd: ngx_pagespeed-release-1.10.33.2-beta/: Bestand of map bestaat niet
hans@vps:$ cd release-${NPS_VERSION}-beta/
hans@vps:
/release-1.10.33.2-beta$ git checkout release release-${NPS_VERSION}-beta
error: pathspec 'release' did not match any file(s) known to git.
error: pathspec 'release-1.10.33.2-beta' did not match any file(s) known to git.
hans@vps:/release-1.10.33.2-beta$ git checkout release ngx_pagespeed-release-${NPS_VERSION}-beta
error: pathspec 'release' did not match any file(s) known to git.
error: pathspec 'ngx_pagespeed-release-1.10.33.2-beta' did not match any file(s) known to git.
hans@vps:
/release-1.10.33.2-beta$ git checkout release v${NPS_VERSION}-beta
error: pathspec 'release' did not match any file(s) known to git.
error: pathspec 'v1.10.33.2-beta' did not match any file(s) known to git.
hans@vps:/release-1.10.33.2-beta$ git checkout release ${NPS_VERSION}-beta
error: pathspec 'release' did not match any file(s) known to git.
error: pathspec '1.10.33.2-beta' did not match any file(s) known to git.
hans@vps:
/release-1.10.33.2-beta$

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 22, 2016

Whoops, I had a typo. Instead of:

git checkout release release-${NPS_VERSION}-beta

you should have

git checkout release-${NPS_VERSION}-beta
@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 22, 2016

Good news: I love those cherry-picks! 😄
Up & running and working without problems.

$ curl -I https://www.fundays.nl/
HTTP/1.1 302 Moved Temporarily
Server: nginx
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
Keep-Alive: timeout=300
Set-Cookie: wb_4647-sid=ee21ngv729h327gajr1162gij2; path=/; HttpOnly
Location: https://www.fundays.nl/pages/fundaysfestival/fundays-live.php
Access-Control-Allow-Origin: *
Strict-Transport-Security: max-age=31536000;
Date: Fri, 22 Jan 2016 21:31:04 GMT
X-Page-Speed: 1.10.33.2-7600
Cache-Control: max-age=0, no-cache

In Safari, Wordpress logins and all the other stuff is also working again like it should.
Good to go! 😃

@crowell

This comment has been minimized.

Copy link
Contributor

crowell commented Jan 22, 2016

the git checkout command should be

git checkout release-1.10.33.2-beta

On Fri, Jan 22, 2016 at 4:06 PM, Hans van Eijsden notifications@github.com
wrote:

No need to say Sorry Jeff. I never asked about it before. 😉

Unfortunately I encounter some errors. I tried to correct them as far as
possible, but I'm stuck at doing the right git checkout. Here are the steps
I've done (system language is in Dutch, nl_NL, sorry):

hans@vps:$ cd
hans@vps:
$ NPS_VERSION=1.10.33.2
hans@vps:$ git clone git@github.com:pagespeed/ngx_pagespeed.git
Cloning into 'ngx_pagespeed'...
Permission denied (publickey).
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
hans@vps:
$ git clone https://github.com/pagespeed/ngx_pagespeed.git
Cloning into 'ngx_pagespeed'...
remote: Counting objects: 8552, done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 8552 (delta 0), reused 0 (delta 0), pack-reused 8547
Receiving objects: 100% (8552/8552), 41.63 MiB | 15.97 MiB/s, done.
Resolving deltas: 100% (4139/4139), done.
Checking connectivity... done.
hans@vps:$ mv ngx_pagespeed release-${NPS_VERSION}-beta
hans@vps:
$ cd ngx_pagespeed-release-${NPS_VERSION}-beta/
-bash: cd: ngx_pagespeed-release-1.10.33.2-beta/: Bestand of map bestaat
niet
hans@vps:$ cd release-${NPS_VERSION}-beta/
hans@vps:
/release-1.10.33.2-beta$ git checkout release
release-${NPS_VERSION}-beta
error: pathspec 'release' did not match any file(s) known to git.
error: pathspec 'release-1.10.33.2-beta' did not match any file(s) known
to git.
hans@vps:/release-1.10.33.2-beta$ git checkout release
ngx_pagespeed-release-${NPS_VERSION}-beta
error: pathspec 'release' did not match any file(s) known to git.
error: pathspec 'ngx_pagespeed-release-1.10.33.2-beta' did not match any
file(s) known to git.
hans@vps:
/release-1.10.33.2-beta$ git checkout release
v${NPS_VERSION}-beta
error: pathspec 'release' did not match any file(s) known to git.
error: pathspec 'v1.10.33.2-beta' did not match any file(s) known to git.
hans@vps:/release-1.10.33.2-beta$ git checkout release
${NPS_VERSION}-beta
error: pathspec 'release' did not match any file(s) known to git.
error: pathspec '1.10.33.2-beta' did not match any file(s) known to git.
hans@vps:
/release-1.10.33.2-beta$


Reply to this email directly or view it on GitHub
#1099 (comment)
.

Jeff Crowell
https://github.com/crowell

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Jan 22, 2016

@jeffkaufman I think we should test for duplicates of some headers so we make sure we don't regress, some of the header manipulation in nginx is excessively tricky. Should I create a new issue for that?

@crowell

This comment has been minimized.

Copy link
Contributor

crowell commented Jan 22, 2016

awesome, thanks for testing!

On Fri, Jan 22, 2016 at 4:31 PM, Hans van Eijsden notifications@github.com
wrote:

Good news: I love those cherry-picks! 😄
Up & running and working without problems.

$ curl -I https://www.fundays.nl/
HTTP/1.1 302 Moved Temporarily
Server: nginx
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
Keep-Alive: timeout=300
Set-Cookie: wb_4647-sid=ee21ngv729h327gajr1162gij2; path=/; HttpOnly
Location: https://www.fundays.nl/pages/fundaysfestival/fundays-live.php
Access-Control-Allow-Origin: *
Strict-Transport-Security: max-age=31536000;
Date: Fri, 22 Jan 2016 21:31:04 GMT
X-Page-Speed: 1.10.33.2-7600
Cache-Control: max-age=0, no-cache

Good to go! 😃


Reply to this email directly or view it on GitHub
#1099 (comment)
.

Jeff Crowell
https://github.com/crowell

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 25, 2016

Fixed by 059dd20

@macbobby

This comment has been minimized.

Copy link

macbobby commented Mar 3, 2016

Do have to clear pagespeed Filecache that the fix is working for me?

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Mar 3, 2016

@macbobby I do not think so, what kind of trouble are you having? Do you have a url?

@macbobby

This comment has been minimized.

Copy link

macbobby commented Mar 3, 2016

No, not atm. But still having to location header lines
double_location_headers

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Mar 3, 2016

@macbobby which ngx_pagespeed & nginx version? release-1.10.33.5-beta was confirmed multiple times to have fixed the issue, but perhaps you are running into another route where you end up with multiple location headers?

@macbobby

This comment has been minimized.

Copy link

macbobby commented Mar 3, 2016

OK, had a bug while downloading correct version. Worked vor me now.

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Mar 3, 2016

@macbobby Thanks for confirming

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment