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

Access to remote addr and httpversion #38

Closed
imp opened this issue Dec 24, 2016 · 7 comments
Closed

Access to remote addr and httpversion #38

imp opened this issue Dec 24, 2016 · 7 comments
Labels
feedback wanted User feedback is needed request Request for new functionality
Milestone

Comments

@imp
Copy link

imp commented Dec 24, 2016

Rocket hides some information about the request when translating hyper request data into its own. Namely remote addr and httpversion are being dropped. These items may be quite useful. Please make them available in the rocket::Request along with the other data.

@SergioBenitez
Copy link
Member

What's the use case for knowing the HTTP version? I'll look into making the remote_addr available.

@SergioBenitez SergioBenitez added the request Request for new functionality label Dec 24, 2016
@imp
Copy link
Author

imp commented Dec 24, 2016

Well, I agree that HTTP version may not appear immediately useful. However, in general hiding this kind of information gives you little, while exposing as much as possible will definitely give your users things to experiment with.

@SergioBenitez
Copy link
Member

SergioBenitez commented Dec 24, 2016

I believe in keeping the API as clean and simple as possible. If there's something that's unused 99% of the time with little reason to use it the rest, I don't believe that feature should be included. That may or may not be the case with exposing the HTTP version - I've personally never had a need for it. Still, I'll tag this "feedback wanted"; perhaps someone else has a good reason to have it exposed.

@SergioBenitez SergioBenitez added the feedback wanted User feedback is needed label Dec 24, 2016
@chris-morgan
Copy link

Remote address is definitely useful. Many types of apps want to record the remote IP address for audit logs.

It’s also quite plausible that you might want to implement things differently in HTTP/1.1 and HTTP/2.0, e.g. bundle things up for HTTP/1.1 or use server push of more smaller resources in HTTP/2.0.

@SergioBenitez
Copy link
Member

I have no qualms about exposing the remote address. I'm looking for use cases on exposing the HTTP version. I'm not sure returning different responses depending on the HTTP version is something I want Rocket to encourage or allow.

@SergioBenitez SergioBenitez added this to the 0.2.0 milestone Jan 3, 2017
@SergioBenitez
Copy link
Member

Since no one has chimed in about a reason to expose the HTTP version, it will not be exposed.

@mrkishi
Copy link

mrkishi commented Feb 5, 2017

e.g. bundle things up for HTTP/1.1 or use server push of more smaller resources in HTTP/2.0.

Why wouldn't you want to encourage this? Is there a better way to deal with bundled/unbundled resources in Rocket already?

SergioBenitez added a commit that referenced this issue Apr 19, 2017
This commit also includes the following changes:

  * `FromRequest` for `SocketAddr` implemented: extracts remote address.
  * All built-in `FromRequest` implementations are documented.
  * Request preprocessing overrides remote IP with value from X-Real-IP header.
  * `MockRequest` allows setting the remote address with `remote()`.

Resolves #38.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback wanted User feedback is needed request Request for new functionality
Projects
None yet
Development

No branches or pull requests

4 participants