Feature #6546
Net::HTTP to check for HTTP_PROXY environment setting.
Description
nahi (Hiroshi Nakamura) suggested I request the OS environment HTTP_PROXY be honoured.
Open-uri checks for this environment setting and acts according whereas Net::HTTP does not. Not having Net::HTTP check for this condition sets precedence and stops the implementation of other environment settings (such as JRuby and JVM -Dhttp.proxyHost).
Having this functionality will greatly enable testing with proxies rather then monkey patching or rewriting to always use HTTP.Proxy.
Files
Related issues
Updated by naruse (Yui NARUSE) over 8 years ago
- Status changed from Open to Feedback
I agree the concept, a patch is welcome.
Updated by drbrain (Eric Hodel) over 8 years ago
- File net.http.proxy_from_env.patch net.http.proxy_from_env.patch added
- Category set to lib
- Assignee set to naruse (Yui NARUSE)
- Target version set to 2.0.0
Here is a patch, it is based on code from net-http-persistent.
It tries both http_proxy and HTTP_PROXY to support windows users (who sometimes use the lowercase version).
Updated by drbrain (Eric Hodel) over 8 years ago
I did not add it to the patch, but if this patch is accepted should Net::HTTP.new be changed so that p_addr = :ENV by default?
This would allow users to use their proxy by default.
Updated by naruse (Yui NARUSE) over 8 years ago
drbrain (Eric Hodel) wrote:
I did not add it to the patch, but if this patch is accepted should Net::HTTP.new be changed so that p_addr = :ENV by default?
This would allow users to use their proxy by default.
The patch still needs a fix to use HTTP_PROXY, so it should be.
But setting password through environment variable is not acceptable,
because they are visible from other users on some OSs as #4388 says (in Japanese).
Updated by drbrain (Eric Hodel) over 8 years ago
OK.
This updated patch uses :ENV by default in Net::HTTP.new and Net::HTTP::Proxy.
This updated patch does not use HTTP_PROXY_USER, etc.
Updated by naruse (Yui NARUSE) over 8 years ago
- Status changed from Feedback to Assigned
- Assignee changed from naruse (Yui NARUSE) to drbrain (Eric Hodel)
drbrain (Eric Hodel) wrote:
OK.
This updated patch uses :ENV by default in Net::HTTP.new and Net::HTTP::Proxy.
This updated patch does not use HTTP_PROXY_USER, etc.
OK, please commit it.
Updated by akr (Akira Tanaka) over 8 years ago
Do you consider that HTTP_PROXY environment variable can be set from outside in CGI by
Proxy: header?
In that case, HTTP_PROXY should be ignored.
Also, platforms which environment variables are case insensitive (Windows), http_proxy is also dangerous.
Updated by drbrain (Eric Hodel) over 8 years ago
- File net.http.proxy_from_env.2.no_env_by_default.patch net.http.proxy_from_env.2.no_env_by_default.patch added
Here is a patch which does not use HTTP_PROXY by default (most of the changes are to documentation).
Please tell me which patch I should apply.
Updated by akr (Akira Tanaka) over 8 years ago
net.http.proxy_from_env.2.no_env_by_default.patch seems have following line.
env_proxy = ENV['http_proxy'] || ENV['HTTP_PROXY']
It may refer the environment variable set by Proxy: header sent from client in CGI.
I think it is vulnerable, as I said in [ruby-core:45579].
As far as I know, this issue is found for libwww-perl in 2001.
http://cpansearch.perl.org/src/GAAS/libwww-perl-6.04/Changes :
|2001-03-14 Gisle Aas gisle@ActiveState.com
|
| Release 5.51
|
| SECURITY FIX: If LWP::UserAgent::env_proxy is called in a CGI
| environment, the case-insensitivity when looking for "http_proxy"
| permits "HTTP_PROXY" to be found, but this can be trivially set by the
| web client using the "Proxy:" header. The fix applied is that
| $ENV{HTTP_PROXY} is not longer honored for CGI scripts.
| The CGI_HTTP_PROXY environment variable can be used instead.
| Problem reported by Randal L. Schwartz.
Updated by nahi (Hiroshi Nakamura) over 8 years ago
drbrain (Eric Hodel) lib/open-uri.rb has battle-tested proxy configuration logic in it. If we really want to add proxy configuration to net/http, we'd better start from the logic.
Updated by drbrain (Eric Hodel) over 8 years ago
=begin
Here is an updated patch that uses the battle-tested code from open-uri. This patch reads the proxy from ((|ENV|)) by default like open-uri. It is safe in a CGI environment just like open-uri.
Details:
URI::Generic#find_proxy was moved from lib/open-uri.rb to lib/uri/generic.rb.
Proxy configuration is now stored in the Net::HTTP instance, not the class. This allows ((|no_proxy|)) to work at connect time.
For compatibility, Net::HTTP.Proxy still returns an anonymous subclass of Net::HTTP, but this subclass only stores proxy configuration information. When new is called the proxy configuration is copied from the class to the instance.
Removed use of Net::HTTP::ProxyDelta. The module still exists for use with lib/net/http/backward.rb.
Net::HTTP.new no longer calls Net::HTTP.Proxy.
Net::HTTP.newobj has been removed.
Net::HTTP#connect no longer calls #conn_address or #conn_port. Instead it calls the new method #proxy?. If the proxy information is being determined from ((|ENV|)) and the current connection is listed in ((|no_proxy|)) a direct connection is made, if it is not listed a proxy connection is made. If a manual proxy has been configured a proxy connection is made. If no proxy is configured a direct connection is made.
#proxy_address and #proxy_port may return nil if called when #proxy? returns false. Since these methods were previously called only via ProxyDelta and conn_address and conn_port I don't think this should be a problem.
#conn_address and #conn_port always return #address and #port now. Since these methods are private I don't think this should be a problem.
#edit_path has been updated to behave properly based on #proxy? and #use_ssl?
I have not altered open-uri to remove it's built-in proxy handling.
I ran the tests with ((|http_proxy|)) pointing to a valid and invalid proxy with no problem.
=end
Updated by drbrain (Eric Hodel) over 8 years ago
- Assignee changed from drbrain (Eric Hodel) to naruse (Yui NARUSE)
Updated by drbrain (Eric Hodel) over 8 years ago
- Status changed from Assigned to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r36476.
Jacob, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
- lib/net/http.rb: Net::HTTP now automatically detects and uses proxies from the environment. A proxy may also be specified as before.
Net::HTTP::Proxy still creates anonymous classes, but these classes
are only used to store configuration information. When an HTTP
instance is created the configuration is now copied.
Additionally, Net::HTTP::ProxyDelta is no longer used by Net::HTTP
[Feature #6546]
- lib/open-uri.rb: Moved URI::Generic#find_proxy to uri/generic.
- lib/uri/generic.rb: Imported find_proxy from open-uri.
- test/open-uri/test_open-uri.rb: Moved proxy-discovery tests to URI.
- test/uri/test_generic.rb: Imported proxy-discovery tests from open-uri.
- test/net/http/test_http.rb: Added tests for proxy behavior.
Updated by shyouhei (Shyouhei Urabe) over 4 years ago
- Has duplicate Feature #12921: Retrieve user and password for proxy from env added