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

Fixes #368 Updates javadoc for getRemoteAddr #370

Merged
merged 2 commits into from
Oct 13, 2020

Conversation

gregw
Copy link
Contributor

@gregw gregw commented Oct 12, 2020

I ended up needing to fix a bit more for #368:

  • reference to RFC 7239 for obtaining local/remote IP and ports
  • removed historic references to CGI variables
  • relaxed the dependence on the host header, which may not be present in HTTP/2
  • removed the reference to dotted IP address form (no longer valid for IPv6)

I'm wondering if we should also say anything about [] around IPv6?

Fixes #368 with javadoc updates for:
 + reference to RFC 7239  for obtaining local/remote IP and ports
 + removed historic references to CGI variables
 + relaxed the dependence on the host header, which may not be present in HTTP/2
@gregw gregw linked an issue Oct 12, 2020 that may be closed by this pull request
Signed-off-by: gregw <gregw@webtide.com>
Copy link
Contributor

@markt-asf markt-asf left a comment

Choose a reason for hiding this comment

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

While it does seem odd to have references to CGI variables in the Javadoc, I have had to rely on CGI spec references in the past to determine correct behaviour. I'm happy for the references to be removed on the understanding that there is no intention to change required behaviour.

@stuartwdouglas stuartwdouglas merged commit 77c4f80 into master Oct 13, 2020
@gregw
Copy link
Contributor Author

gregw commented Oct 14, 2020

@markt-asf I removed the CGI references as they were written over a decade ago and who knows if CGI has been update/changed/refined in that time... nor given the RFC7239 nuances we have discussed if they will always be the same value. Thus it felt like it was not worth anything as part of the specification, nor could we really say it was an invariant.

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.

Impl Check: ServletRequest.getRemoteAddr and intermediaries
3 participants