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

fastcgi: Comply with RFC3875, ensure a leading slash on index rewrite #3570

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

francislavoie
Copy link
Member

@francislavoie francislavoie commented Jul 11, 2020

Closes #3568
Closes #3569

See https://tools.ietf.org/html/rfc3875#section-4.1.13 for SCRIPT_NAME requiring leading slash
See https://tools.ietf.org/html/rfc3875#section-4.1.15 for SERVER_PORT requiring omission if empty

@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 francislavoie requested a review from mholt July 11, 2020 03:42
@francislavoie francislavoie force-pushed the fastcgi-rfc-compliance branch from 8c544ba to 86c4713 Compare July 13, 2020 23:23
@francislavoie francislavoie requested a review from mholt July 13, 2020 23:33
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

LGTM, thank you very much again Francis!

@mholt mholt merged commit 0665a86 into caddyserver:master Jul 17, 2020
@francislavoie francislavoie deleted the fastcgi-rfc-compliance branch July 17, 2020 21:41
@ttys3
Copy link
Contributor

ttys3 commented Feb 12, 2022

the latest implementation has a problem.

see

this makes it optional, not MUST be set like rfc said.

	reqHost, reqPort, err := net.SplitHostPort(r.Host)
	if err != nil {
		// whatever, just assume there was no port
		reqHost = r.Host
	}
	
	
	// compliance with the CGI specification requires that
	// SERVER_PORT should only exist if it's a valid numeric value.
	// Info: https://www.ietf.org/rfc/rfc3875 Page 18
	if reqPort != "" {
		env["SERVER_PORT"] = reqPort
	}

as https://tools.ietf.org/html/rfc3875#section-4.1.15

4.1.15.  SERVER_PORT

   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.

Note that this variable MUST be set

I fired up a PR #4572

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
3 participants