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

Don’t set SERVER_PORT to empty string (reverse proxy) #3568

Closed
sorbits opened this issue Jul 11, 2020 · 5 comments · Fixed by #3570
Closed

Don’t set SERVER_PORT to empty string (reverse proxy) #3568

sorbits opened this issue Jul 11, 2020 · 5 comments · Fixed by #3570
Labels
bug 🐞 Something isn't working
Milestone

Comments

@sorbits
Copy link

sorbits commented Jul 11, 2020

I ran into an issue with a PHP framework that got confused about SERVER_PORT being set to an empty string.

Assuming this framework has been used with many other web servers, it implies that the de facto standard is to either have a value, or not set it at all, which I therefore propose that Caddy does as well.

That said, I did open an issue with the PHP framework in question (bcosca/fatfree#1194) as I believe their code should be updated, but the problem may also affect other frameworks, which is why I am also opening an issue with Caddy.

@sorbits
Copy link
Author

sorbits commented Jul 11, 2020

Update: I checked RFC3875 which makes it clear that Caddy must set SERVER_PORT to a value:

The SERVER_PORT variable MUST be set to the TCP/IP port number on
which this request is received from the client.  This value is used
in the port part of the Script-URI.

    SERVER_PORT = server-port
    server-port = 1*digit

Note that this variable MUST be set, even if the port is the default
port for the scheme and could otherwise be omitted from a URI.

@francislavoie
Copy link
Member

francislavoie commented Jul 11, 2020

Interesting.

@mholt I think we should look at r.URL and guess the port based on the scheme if it can't be split from r.Host maybe?

https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go#L223

@francislavoie francislavoie added the bug 🐞 Something isn't working label Jul 11, 2020
@francislavoie francislavoie added this to the v2.2.0 milestone Jul 11, 2020
@francislavoie
Copy link
Member

francislavoie commented Jul 11, 2020

Actually wait, what's your Caddyfile? Are you using a unix socket for php_fastcgi? In that case there's no server port. If you're using a TCP socket (i.e. port 9000 for php-fpm typically) then that's what SERVER_PORT would be, I think.

If that's the case, we can skip that env for compliance.

@francislavoie
Copy link
Member

#3570 should fix both of the issues, I think.

Could you give it a shot? You can use the CI artifact from here: https://github.com/caddyserver/caddy/actions/runs/165314958

@sorbits
Copy link
Author

sorbits commented Jul 11, 2020

#3570 should fix both of the issues, I think.

Awesome, thanks!

I can confirm that it fixes both issues, furthermore you are correct that I am using a unix socket for FastCGI (so there is no port).

@sorbits sorbits closed this as completed Jul 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants