Feature #6546

Net::HTTP to check for HTTP_PROXY environment setting.

Added by Jacob Evans almost 2 years ago. Updated over 1 year ago.

[ruby-core:45426]
Status:Closed
Priority:Normal
Assignee:Yui NARUSE
Category:lib
Target version:2.0.0

Description

@nahi 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.

net.http.proxy_from_env.patch Magnifier (5.95 KB) Eric Hodel, 06/09/2012 10:00 AM

net.http.proxy_from_env.2.patch Magnifier (8.01 KB) Eric Hodel, 06/12/2012 09:17 AM

net.http.proxy_from_env.2.no_env_by_default.patch Magnifier - Patch 2 without default of p_addr = :ENV (7.42 KB) Eric Hodel, 06/13/2012 06:36 AM

net.http.proxy_from_env.3.patch Magnifier - Patch using proxy detection from open-uri (23.6 KB) Eric Hodel, 06/28/2012 09:42 AM


Related issues

Related to ruby-trunk - Bug #4388: open-uriで環境変数http_proxyを使うときに認証付きのProxyが使えません Rejected 02/10/2011
Related to ruby-trunk - Bug #8771: Start does not use proxy configuration form ENV variables Rejected 08/11/2013

Associated revisions

Revision 36476
Added by Eric Hodel over 1 year ago

  • 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.

Revision 36477
Added by Eric Hodel over 1 year ago

  • NEWS: Updated net/http for automatic proxy detection (#6546) and automatic gzip and deflate compression (#6492, #6494).

History

#1 Updated by Yui NARUSE almost 2 years ago

  • Status changed from Open to Feedback

I agree the concept, a patch is welcome.

#2 Updated by Eric Hodel almost 2 years ago

Here is a patch, it is based on code from net-http-persistent.

It tries both httpproxy and HTTPPROXY to support windows users (who sometimes use the lowercase version).

#3 Updated by Eric Hodel almost 2 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.

#4 Updated by Yui NARUSE almost 2 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).

#5 Updated by Eric Hodel almost 2 years ago

OK.

This updated patch uses :ENV by default in Net::HTTP.new and Net::HTTP::Proxy.

This updated patch does not use HTTPPROXYUSER, etc.

#6 Updated by Yui NARUSE almost 2 years ago

  • Status changed from Feedback to Assigned
  • Assignee changed from Yui NARUSE to 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 HTTPPROXYUSER, etc.

OK, please commit it.

#7 Updated by Akira Tanaka almost 2 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.

#8 Updated by Eric Hodel almost 2 years ago

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.

#9 Updated by Akira Tanaka almost 2 years ago

net.http.proxyfromenv.2.noenvby_default.patch seems have following line.

envproxy = ENV['httpproxy'] || 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 .

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::envproxy is called in a CGI
| environment, the case-insensitivity when looking for "http
proxy"
| permits "HTTPPROXY" 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 CGIHTTPPROXY environment variable can be used instead.
| Problem reported by Randal L. Schwartz.

#10 Updated by Hiroshi Nakamura almost 2 years ago

@drbrain 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.

#11 Updated by Eric Hodel almost 2 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 #connaddress or #connport. 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.

#proxyaddress and #proxyport may return nil if called when #proxy? returns false. Since these methods were previously called only via ProxyDelta and connaddress and connport I don't think this should be a problem.

#connaddress and #connport always return #address and #port now. Since these methods are private I don't think this should be a problem.

#editpath has been updated to behave properly based on #proxy? and #usessl?

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

#12 Updated by Eric Hodel almost 2 years ago

  • Assignee changed from Eric Hodel to Yui NARUSE

#13 Updated by Yui NARUSE over 1 year ago

OK, commit it.

#14 Updated by Eric Hodel over 1 year 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.

Also available in: Atom PDF