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

Fix get_absolute_root_url() to detect secure connection #914

Closed
wants to merge 2 commits into from

Conversation

sbias
Copy link

@sbias sbias commented Aug 13, 2018

if a reverse proxy is used

{
if (
(isset($_SERVER['HTTPS']) && ((strtolower($_SERVER['HTTPS']) == 'on') or ($_SERVER['HTTPS'] == 1)))
|| (isset($_SERVER['HTTP_X_FORWARDED_PROTO']) && strtolower($_SERVER['HTTP_X_FORWARDED_PROTO']) == 'https')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should mix or and ||. I think the Piwigo standard is to favour the former.

@choenig
Copy link

choenig commented Apr 13, 2020

Hi,

I'd really like to get this applied :-)

I'm new to Piwigo and I almost gave up setting it up. It took me half a day to find this issue as I'm running Piwigo behind a traefik2 https-proxy. Maybe this would also fix #1128. There someone describes the same issues I am having (though he does not talk about a proxy-setup).

Just for completenes:
My problem is, that i.php returns an 'http'-url when mass-creating thumbnails. My browser rejects that with 'blocked:mixed-content' and so the thumbnail-creation fails.

Br
Christian

@jobec
Copy link

jobec commented Aug 2, 2020

@samwilson
Any chance on getting this merged? X-Forwarded-Proto is a defacto standard header for these kind of things.

I personally needed to patch the code while running piwigo inside docker (apache + php) on port 8080 with Caddy in front as reverse proxy for SSL. Piwigo should not try to build the full path to anything and just stick with relative paths and let the browser handle it.

@samwilson
Copy link
Contributor

@jobec I'm not a maintainer here, just a random user. My experience has been that PRs are slow to move.

@nika1001
Copy link

I would also like this issue to be resolved, we use a proxy and its really not a nice solution to patch the code each time a new update comes out.

@jobec
Copy link

jobec commented Aug 11, 2020

Meanwhile, I bypassed the issue by adding this to my config.inc.php file:

// Support X-Forwarded-Proto header for HTTPS detection
if ( $_SERVER['HTTP_X_FORWARDED_PROTO'] == 'https' ) {
    $_SERVER['HTTPS'] = 'on';
}

That way I don't need to mess around with piwigo's code but still have it respond to the defacto X-Forwarded-Proto header.

@GunoH
Copy link

GunoH commented Oct 31, 2020

// Support X-Forwarded-Proto header for HTTPS detection
if ( $_SERVER['HTTP_X_FORWARDED_PROTO'] == 'https' ) {
$_SERVER['HTTPS'] = 'on';
}

I also had to add $_SERVER['SERVER_PORT'] = 443; here, otherwise the get_absolute_root_url() function would insert :80 into the https url.

@ambs
Copy link

ambs commented Aug 11, 2021

I did not required the SERVER_PORT setting. But the check for X-Forwarded-Proto is a must, and please, should be merged asap.

@russell
Copy link

russell commented Aug 28, 2021

the server port is probably also available as the variable X-Forwarded-Port so if that is set it could be used to avoid needing to set the port manually to 443

@jobec thank you for that hack, you have saved me some time.

@plegall
Copy link
Member

plegall commented Jul 21, 2022

I close this pull-request, as a duplicate of #483

@plegall plegall closed this Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants