Allow to send proxy params.

[ryanfox1985]

Hello,

according with this documentation: https://www.rubydoc.info/stdlib/net/Net%2FHTTP.start

The opts parameters are not send in a correct order, also is not allowed to request though a proxy.

Regards.

[gaffneyc]

@ryanfox1985 Thank you for the pull request.

Is there a reason you're unable to use the http_proxy environment variable and need it implemented at the client level? Want to make sure I understand before moving forward.

[ryanfox1985]

@gaffneyc is not possible to use the http_proxy because right now is not calling with the good parameters to the method start.

Here you have the link to the documentation https://www.rubydoc.info/stdlib/net/Net%2FHTTP.start to review.

The propose of this PR is resolve this issue with the parameters.

[ryanfox1985]

the current code is overriding the p_addr => HTTP.start(address, port=nil, p_addr=:ENV, p_port=nil, p_user=nil, p_pass=nil, opt, &block)

And removes the default value :ENV

[ryanfox1985]

My proposal fixes the parameters order and allows more flexibility to configure the proxy passing options.

[gaffneyc]

Trying to find documentation for it but I believe Ruby will automatically use the http_proxy environment variable when set even if you don't pass a param to HTTP.start

[gaffneyc]

@ryanfox1985 Here is the documentation https://ruby-doc.org/stdlib-2.7.1/libdoc/net/http/rdoc/Net/HTTP.html#class-Net::HTTP-label-Proxies

It seems like you should be able to do this to use a proxy:

export http_proxy=http://your.proxy:port
ruby script.rb

[ryanfox1985]

@gaffneyc not working your proposal with the current code:

Net::HTTP.start(uri.host, uri.port, opts) do |http|

...

opts = hash { with timeouts... }

The current code overrides the parameter p_addr (with a default value :ENV) for opts that is a hash and its the 7th parameter, look the api doc for #start method.

HTTP.start(address, port=nil, p_addr=:ENV, p_port=nil, p_user=nil, p_pass=nil, opt, &block)

https://www.rubydoc.info/stdlib/net/Net%2FHTTP.start

[ryanfox1985]

The current code sends p_addr = opts, it's wrong

[gaffneyc]

@ryanfox1985 The signature for HTTP.start changes based on the number of arguments passed in. It will always treat the last item in the list, if it's a Hash, as the opt param. I've included the source below. On the first line it checks if the last argument is a hash and extracts it from the argument list.

At least in this case, the call to HTTP.start is correct and does work as is.

The question I have: for your use case do you need to pass in the proxy params?

def HTTP.start(address, *arg, &block)
  arg.pop if opt = Hash.try_convert(arg[-1])
  port, p_addr, p_port, p_user, p_pass = *arg
  p_addr = :ENV if arg.size < 2
  port = https_default_port if !port && opt && opt[:use_ssl]
  http = new(address, port, p_addr, p_port, p_user, p_pass)
  http.ipaddr = opt[:ipaddr] if opt && opt[:ipaddr]

  if opt
    if opt[:use_ssl]
      opt = {verify_mode: OpenSSL::SSL::VERIFY_PEER}.update(opt)
    end
    http.methods.grep(/\A(\w+)=\z/) do |meth|
      key = $1.to_sym
      opt.key?(key) or next
      http.__send__(meth, opt[key])
    end
  end

  http.start(&block)
end

[gaffneyc]

@ryanfox1985 I mentioned the http_proxy environment variable and wanted to test it to make sure it works.

Using tinyproxy I was able to set up a local HTTP proxy that logged requests. (Using tinyproxy -d).

I then ran the below code to test that http_proxy worked and was able to see requests going through the proxy.

http_proxy=http://127.0.0.1:8888 DMS_API_KEY=[key here] ruby examples/api.rb

I was then able to see in the tinyproxy logs that it was connecting through the proxy

CONNECT   Apr 29 14:24:43 [15325]: Connect (file descriptor 10): localhost [127.0.0.1]
CONNECT   Apr 29 14:24:43 [15325]: Request (file descriptor 10): CONNECT api.deadmanssnitch.com:443 HTTP/1.1
INFO      Apr 29 14:24:43 [15325]: No upstream proxy for api.deadmanssnitch.com
INFO      Apr 29 14:24:43 [15325]: opensock: opening connection to api.deadmanssnitch.com:443
INFO      Apr 29 14:24:43 [15325]: opensock: getaddrinfo returned for api.deadmanssnitch.com:443
CONNECT   Apr 29 14:24:43 [15325]: Established connection to host "api.deadmanssnitch.com" using file descriptor 11.
INFO      Apr 29 14:24:43 [15325]: Not sending client headers to remote machine
INFO      Apr 29 14:24:44 [15325]: Closed connection between local client (fd:10) and remote client (fd:11)

[ryanfox1985]

@gaffneyc I reviewed the source and you are right.

The first line removes the last argument if it's a hash: arg.pop if opt = Hash.try_convert(arg[-1]) and this line forces the p_addr => p_addr = :ENV if arg.size < 2 because the only argument is the port.

Finally I discovered my problem, we have ruby 2.1.3 and the net library not sets p_addr, leaves as a nil and not as a :ENV:

Here you can check the source => https://apidock.com/ruby/v2_1_10/Net/HTTP/start/class

[gaffneyc]

@ryanfox1985 Hm... okay. Ruby 2.1 is really old at this point. Are you using Snitcher inside an application or from the command line with the system default ruby?

[gaffneyc]

Found the feature request for http_proxy support and it looks like it should work for Ruby 2.0 and newer. I will see if I can test it against Ruby 2.1.

[gaffneyc]

After some more testing it looks like the http_proxy environment variable wasn't supported until Ruby 2.5