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

net/http/cgi: Remote Port Param incorrectly discarded #8351

Closed
georgyo opened this issue Jul 9, 2014 · 6 comments
Closed

net/http/cgi: Remote Port Param incorrectly discarded #8351

georgyo opened this issue Jul 9, 2014 · 6 comments
Milestone

Comments

@georgyo
Copy link
Contributor

georgyo commented Jul 9, 2014

What does 'go version' print?
go version devel +eae7f816eb6c Tue Jul 08 18:41:07 2014 -0400 linux/amd64

What happened?
The remote connection port is always zero when using CGI or FCGI handlers.

What should have happened instead?
Apache 2.4, and possibly others set the REMOTE_PORT field correctly, however cgi package
hard codes it to zero. It should first check if it has the required param before
defaulting to zero.

net/http/fcgi uses cgi.RequestFromMap, and therefor is also affected.

A simple patch is attached.

Attachments:

  1. remote_port.patch (707 bytes)
@georgyo
Copy link
Contributor Author

georgyo commented Jul 10, 2014

Comment 1:

Reading though other's people issues, I realized that I should have sent the patch to
codereview.  That is now done.
https://golang.org/cl/112900044/

@georgyo
Copy link
Contributor Author

georgyo commented Jul 14, 2014

Comment 2:

Last comment on this, I just went though the CGI RFC (rfc3875) and REMOTE_PORT is not
specified in the spec.
That said, I can't find a single server that does not set REMOTE_PORT by default. Apache
2.2, Apache 2.4, nginx, and lighttpd all set REMOTE_PORT.
I originally dug deeper to see if we could remove the logic of setting the port to zero
if REMOTE_PORT was unset, but because the spec does not mandate it, we must do the check.
Lastly, while it is generally useless to criticize a finalized RFC, I find it very odd
that the CGI spec specifies both REMOTE_ADDR and REMOTE_HOST. The latter of which wants
a DNS lookup. On top of that I can't find a server that does pass the REMOTE_HOST param,
despite it being something that "SHOULD" be set according to the RFC. As such I can't
find a single widely used webserver that follows the spec religiously.

@griesemer
Copy link
Contributor

Comment 3:

Labels changed: added repo-main.

@georgyo
Copy link
Contributor Author

georgyo commented Dec 16, 2014

Any chance of this making it in 1.5?

@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Dec 16, 2014
@bradfitz
Copy link
Contributor

@georgyo, the tree just opened. See https://groups.google.com/forum/#!topic/golang-dev/otCULnOjs7I and re-send this patch using the new Git-based process. Thanks!

@bradfitz bradfitz removed the new label Dec 18, 2014
@georgyo
Copy link
Contributor Author

georgyo commented Dec 19, 2014

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants