configurable trusted proxies?

23 views
Skip to first unread message

S. Brent Faulkner

unread,
Jan 30, 2016, 10:40:33 AM1/30/16
to Rack Development
Hi all... wondering if anyone is willing to review and/or discuss the idea of configurable trusted proxies for Rack ? https://github.com/rack/rack/pull/1001

Thanks

--Brent

S. Brent Faulkner

Eric Wong

unread,
Feb 1, 2016, 4:53:05 AM2/1/16
to rack-...@googlegroups.com
"S. Brent Faulkner" <sbfau...@gmail.com> wrote:
> Hi all... wondering if anyone is willing to review and/or discuss the idea
> of configurable trusted proxies for Rack
> ? https://github.com/rack/rack/pull/1001

I'm not sure why you put the directive inside Rack::Utils
instead of Rack::Request where it is the only user.

I suspect there's Rack::Utils users who do not use Rack::Request.

Not sure if the extra Array creation and block invocation makes
a meaningful performance difference for anyone.

Otherwise I guess it's fine (not that I can merge your change).


Note: I reviewed by adding the following line to my .git/config
at the remote configured for git://github.com/rack/rack.git

fetch = +refs/pull/*:refs/remotes/pull/*

Then "git fetch", and "git log -p --color-words pull/1001/head"
This was following commit 13397c7963b30f1d05e316d4de1930a6f81abf38
The "--color-words" switch made the test/spec_request.rb changes
much more readable.

(I don't use proprietary APIs or run JavaScript)

S. Brent Faulkner

unread,
Feb 1, 2016, 9:00:39 PM2/1/16
to Rack Development, e...@80x24.org
On Monday, February 1, 2016 at 4:53:05 AM UTC-5, Eric Wong wrote:
"S. Brent Faulkner" <sbfau...@gmail.com> wrote:
> Hi all... wondering if anyone is willing to review and/or discuss the idea
> of configurable trusted proxies for Rack
> ? https://github.com/rack/rack/pull/1001

I'm not sure why you put the directive inside Rack::Utils
instead of Rack::Request where it is the only user.


discussing the configuration in github :-) ... fwiw.. the only other configurable options I saw anywhere were in Utils, so I put this configuration there as well. Also, determining what was/wasn't a trusted proxy sort of felt like a "utility" activity (especially once the config was done there)

I suspect there's Rack::Utils users who do not use Rack::Request.

Not sure if the extra Array creation and block invocation makes
a meaningful performance difference for anyone.


re: performance... I don't think so from my (adhoc) tests, and I've essentially borrowed how it's done in ActionDispatch::RemoteIp ... if this is viewed as a problem we can likely come up with an alternative

Otherwise I guess it's fine (not that I can merge your change).


Note: I reviewed by adding the following line to my .git/config
at the remote configured for git://github.com/rack/rack.git

        fetch = +refs/pull/*:refs/remotes/pull/*

Then "git fetch", and "git log -p --color-words pull/1001/head"
This was following commit 13397c7963b30f1d05e316d4de1930a6f81abf38
The "--color-words" switch made the test/spec_request.rb changes
much more readable.

(I don't use proprietary APIs or run JavaScript)

Thanks for looking! 
Reply all
Reply to author
Forward
0 new messages