Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set proxies according to the *_PROXY environment variables #22

Open
hkalexling opened this issue Aug 7, 2020 · 4 comments
Open

Set proxies according to the *_PROXY environment variables #22

hkalexling opened this issue Aug 7, 2020 · 4 comments

Comments

@hkalexling
Copy link

The [protocol]_PROXY and NO_PROXY environment variables are commonly used to configure proxies in applications. Wget and Emacs are two examples. Python's requests library also respects HTTP_PROXY and HTTPS_PROXY out of the box.

It would be great if we could have a helper method like HTTP::Proxy::Client.use_env that would do the followings

  1. When used, it would automatically load the appropriate settings from ENV and create the corresponding HTTP::Proxy::Client objects.
  2. It would then monkey-patch the HTTP::Client class in the standard library so that the clients use the appropriate proxies in all requests.

Here's an example implementation I used in my project. I will be happy to submit a PR if you think it's a good idea.

@kojix2
Copy link

kojix2 commented Apr 14, 2023

Looks good for me! 👍

However, the get_proxy method could return nil, resulting in the following error

In src/utils/proxy.cr:33:26

 33 | client.set_proxy get_proxy uri
                       ^--------
Error: expected argument #1 to 'HTTP::Client#set_proxy' to be HTTP::Proxy::Client, not (HTTP::Proxy::Client | Nil)

This error can of course be easily avoided

        if prxy = get_proxy(uri)
          client.set_proxy prxy
        end

Thanks!

@mamantoha
Copy link
Owner

Hi @hkalexling @kojix2
I apologize for the delay in addressing this issue. Could you please create a pull request for this? I would greatly appreciate it. Thank you.

@kojix2
Copy link

kojix2 commented Mar 23, 2024

Long time no see.

For some reason, the above code didn't work on Crest.
I haven't looked into why. I wrote a workaround like this:

module Crest
  class Request
    def initialize(
      method : Symbol,
      url : String,
      form = {} of String => String,
      **args
    )
      previous_def
      url = URI.parse(@url)
      no_proxy = ENV["no_proxy"]? || ENV["NO_PROXY"]?
      return if no_proxy && no_proxy.split(",").includes?(url.host)
      key = "#{url.scheme}_proxy"
      proxy_ = ENV[key.downcase]? || ENV[key.upcase]?
      return if proxy_.nil?
      uri = URI.parse proxy_
      @p_addr = uri.host
      @p_port = uri.port
      @p_user = uri.user
      @p_pass = uri.password
      set_proxy!(@p_addr, @p_port, @p_user, @p_pass)
    end
  end
end

However, this workaround breaks as soon as the signature of the Crest::Request#initialize method changes.
It would be better to handle it on the https_proxy side.

@mamantoha
Copy link
Owner

@kojix2 Which specific piece of code are you referring to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants