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

Skip passing port in Host header if standard port. #1362

Merged
merged 1 commit into from Jun 15, 2016

Conversation

karrots
Copy link
Contributor

@karrots karrots commented Jun 15, 2016

Fixes #1361.

  • This PR is compliant with the contributing guidelines (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/en/*.

Rationale: Current HTTP module sends port in Host header even if using standard 80 and 443 ports. This fixes the behavior with the exception of HTTPS on port 80 and HTTP on port 443. A more thorough fix would take a bit of rework of the module.

Committers supporting this PR: leave blank


if ((req->port == 80) || (req->port == 443))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should be "secure" flag check, not just omit both 80s and 443s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did miss there was already a req->secure field in the structure. I'll update the code to check that.

@djphoenix
Copy link
Contributor

IMHO if you point to RFC, you should use all of them :)

@karrots
Copy link
Contributor Author

karrots commented Jun 15, 2016

RFC only says leave it off if default. Never actually says the port should be there when it's not default, though. So it's a bit ambiguous.

Some virtual hosts break if the port is added in the headers.
@marcelstoer marcelstoer merged commit 7ff8326 into nodemcu:dev Jun 15, 2016
@marcelstoer
Copy link
Member

Thanks.

@karrots karrots deleted the http-header-port branch June 15, 2016 13:54
@marcelstoer marcelstoer added this to the 1.5.4 milestone Jul 11, 2016
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.

None yet

3 participants