Feature #6546
closedNet::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
Updated by naruse (Yui NARUSE) over 12 years ago
- Status changed from Open to Feedback
I agree the concept, a patch is welcome.
Updated by drbrain (Eric Hodel) over 12 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 12 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 12 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 12 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 12 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 12 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 12 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 12 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 12 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 12 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 12 years ago
- Assignee changed from drbrain (Eric Hodel) to naruse (Yui NARUSE)
Updated by naruse (Yui NARUSE) over 12 years ago
OK, commit it.
Updated by drbrain (Eric Hodel) over 12 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) almost 8 years ago
- Has duplicate Feature #12921: Retrieve user and password for proxy from env added