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

Add peer and local socket addrs, as well as Request::remote #119

Merged
merged 9 commits into from
May 7, 2020

Conversation

jbr
Copy link
Member

@jbr jbr commented Apr 24, 2020

this is the first PR of three to address http-rs/tide#462

This also adds an initial implementation of Request::remote and Request::forwarded_for. Parsing the Forwarded header value probably would be more elegant with a proper parsing or regex library, but I didn't want to add an additional dependency just for this.

@jbr jbr changed the title add peer and local socket addrs to Request add peer and local socket addrs to Request & Response Apr 24, 2020
@jbr jbr changed the title add peer and local socket addrs to Request & Response Add peer and local socket addrs to Request & Response Apr 24, 2020
@jbr jbr changed the title Add peer and local socket addrs to Request & Response Add peer and local socket addrs, as well as Request::remote Apr 25, 2020
Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

Overall this PR looks really good! A few nits around pub / priv methods, but otherwise 💯

src/request.rs Outdated Show resolved Hide resolved
src/request.rs Outdated Show resolved Hide resolved
src/request.rs Outdated Show resolved Hide resolved
src/response.rs Outdated Show resolved Hide resolved
src/response.rs Outdated Show resolved Hide resolved
@jbr
Copy link
Member Author

jbr commented May 7, 2020

Addressed all changes, thanks for the review!

@jbr jbr requested a review from yoshuawuyts May 7, 2020 19:53
Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

🎉

@yoshuawuyts yoshuawuyts merged commit 6dceef3 into http-rs:master May 7, 2020
@yoshuawuyts yoshuawuyts mentioned this pull request May 15, 2020
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

2 participants