configurable trusted proxies?

[S. Brent Faulkner]

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]

"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]

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!