From 94f1f49a1d1cf04fa2939afd4a00607fc4dfa35a Mon Sep 17 00:00:00 2001 From: Shahne Rodgers Date: Sat, 6 Oct 2018 12:04:03 +1300 Subject: [PATCH 1/3] Added support for the NO_PROXY environment variable. This commit adds support for the NO_PROXY (or no_PROXY / no_proxy) environment variable in a GNU-supported format (https://www.gnu.org/software/emacs/manual/html_node/url/Proxies.html) to fix #331. --- lib/httpoison/base.ex | 41 ++++++++++++ test/httpoison_base_test.exs | 122 +++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+) diff --git a/lib/httpoison/base.ex b/lib/httpoison/base.ex index a61faca..3c4c0eb 100644 --- a/lib/httpoison/base.ex +++ b/lib/httpoison/base.ex @@ -593,6 +593,7 @@ defmodule HTTPoison.Base do "https" -> System.get_env("HTTPS_PROXY") || System.get_env("https_proxy") _ -> nil end + |> check_no_proxy(request_url) end proxy_auth = Keyword.get(options, :proxy_auth) @@ -613,6 +614,46 @@ defmodule HTTPoison.Base do hn_proxy_options end + defp check_no_proxy(nil, _) do + # Don't bother to check no_proxy if there's no proxy to use anyway. + nil + end + + defp check_no_proxy(proxy, request_url) do + request_host = URI.parse(request_url).host + + should_bypass_proxy = get_no_proxy_system_env() + |> String.split(",") + |> Enum.any?(fn domain -> matches_no_proxy_value?(request_host, domain) end) + + if should_bypass_proxy do + nil + else + proxy + end + end + + defp get_no_proxy_system_env() do + System.get_env("NO_PROXY") || System.get_env("no_PROXY") || System.get_env("no_proxy") || + "" + end + + defp matches_no_proxy_value?(request_host, no_proxy_value) do + cond do + no_proxy_value == "" -> false + String.starts_with?(no_proxy_value, ".") -> String.ends_with?(request_host, no_proxy_value) + String.contains?(no_proxy_value, "*") -> matches_wildcard?(request_host, no_proxy_value) + true -> request_host == no_proxy_value + end + end + + defp matches_wildcard?(request_host, wildcard_domain) do + Regex.escape(wildcard_domain) + |> String.replace("\\*", ".*") + |> Regex.compile!() + |> Regex.match?(request_host) + end + @doc false @spec request(atom, atom, binary, body, headers, any, fun, fun, fun) :: {:ok, Response.t() | AsyncResponse.t()} | {:error, Error.t()} diff --git a/test/httpoison_base_test.exs b/test/httpoison_base_test.exs index d791ea3..6176bd5 100644 --- a/test/httpoison_base_test.exs +++ b/test/httpoison_base_test.exs @@ -29,6 +29,9 @@ defmodule HTTPoisonBaseTest do System.delete_env("http_proxy") System.delete_env("HTTPS_PROXY") System.delete_env("https_proxy") + System.delete_env("NO_PROXY") + System.delete_env("no_PROXY") + System.delete_env("no_proxy") end) stub(:hackney) @@ -262,6 +265,125 @@ defmodule HTTPoisonBaseTest do } end + test "having matching no_proxy env variable set with proxy env variable" do + # If the variable is specified directly, no_proxy should be ignored. + System.put_env("NO_PROXY", ".somedomain.com") + + expect(:hackney, :request, fn :post, + "http://www.somedomain.com", + [], + "body", + [proxy: "proxy"] -> + {:ok, 200, "headers", :client} + end) + + expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + + assert HTTPoison.post!("www.somedomain.com", "body", [], proxy: "proxy") == + %HTTPoison.Response{ + status_code: 200, + headers: "headers", + body: "response", + request_url: "http://www.somedomain.com" + } + end + + test "having matching no_proxy env variable set with http_proxy env" do + # If the variable is specified indirectly, no_proxy should be used. + System.put_env("HTTP_PROXY", "proxy") + System.put_env("NO_PROXY", ".somedomain.com") + + expect(:hackney, :request, fn :post, "http://www.somedomain.com", [], "body", [] -> + {:ok, 200, "headers", :client} + end) + + expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + + assert HTTPoison.post!("www.somedomain.com", "body") == + %HTTPoison.Response{ + status_code: 200, + headers: "headers", + body: "response", + request_url: "http://www.somedomain.com" + } + end + + test "having no_proxy env variable set that does not match site" do + System.put_env("HTTP_PROXY", "proxy") + System.put_env("NO_PROXY", ".nonmatching.com") + + expect(:hackney, :request, fn :post, "http://www.somedomain.com", [], "body", [proxy: "proxy"] -> + {:ok, 200, "headers", :client} + end) + + expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + + assert HTTPoison.post!("http://www.somedomain.com", "body") == + %HTTPoison.Response{ + status_code: 200, + headers: "headers", + body: "response", + request_url: "http://www.somedomain.com" + } + end + + test "having no_proxy env variable with multiple domains" do + System.put_env("HTTP_PROXY", "proxy") + System.put_env("NO_PROXY", ".nonmatching.com,.matching.com") + + expect(:hackney, :request, fn :post, "http://www.matching.com", [], "body", [] -> + {:ok, 200, "headers", :client} + end) + + expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + + assert HTTPoison.post!("http://www.matching.com", "body") == + %HTTPoison.Response{ + status_code: 200, + headers: "headers", + body: "response", + request_url: "http://www.matching.com" + } + end + + test "having no_proxy env variable with wildcard domains" do + System.put_env("HTTP_PROXY", "proxy") + System.put_env("NO_PROXY", ".nonmatching.com,*.matching.com") + + expect(:hackney, :request, fn :post, "http://www.matching.com", [], "body", [] -> + {:ok, 200, "headers", :client} + end) + + expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + + assert HTTPoison.post!("http://www.matching.com", "body") == + %HTTPoison.Response{ + status_code: 200, + headers: "headers", + body: "response", + request_url: "http://www.matching.com" + } + end + + test "having no_proxy env variable with non-matching wildcard domains" do + System.put_env("HTTP_PROXY", "proxy") + System.put_env("NO_PROXY", "*.nonmatching.com") + + expect(:hackney, :request, fn :post, "http://www.matching.com", [], "body", [proxy: "proxy"] -> + {:ok, 200, "headers", :client} + end) + + expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + + assert HTTPoison.post!("http://www.matching.com", "body") == + %HTTPoison.Response{ + status_code: 200, + headers: "headers", + body: "response", + request_url: "http://www.matching.com" + } + end + test "passing ssl option" do expect(:hackney, :request, fn :post, "http://localhost", From fdd740221063f4985a73b36ddd489e119420ea96 Mon Sep 17 00:00:00 2001 From: Shahne Rodgers Date: Sat, 6 Oct 2018 14:53:37 +1300 Subject: [PATCH 2/3] Split the common "expect" parts of the proxy tests into a separate method. Hopefully this makes the tests a little easier to understand. --- test/httpoison_base_test.exs | 88 ++++++++++++------------------------ 1 file changed, 28 insertions(+), 60 deletions(-) diff --git a/test/httpoison_base_test.exs b/test/httpoison_base_test.exs index 6176bd5..8da7936 100644 --- a/test/httpoison_base_test.exs +++ b/test/httpoison_base_test.exs @@ -119,12 +119,8 @@ defmodule HTTPoisonBaseTest do end test "passing proxy option" do - expect(:hackney, :request, fn - :post, "http://localhost", [], "body", [proxy: "proxy"] -> {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) - + expect_hackney_post_with_proxy("http://localhost", "proxy") + assert HTTPoison.post!("localhost", "body", [], proxy: "proxy") == %HTTPoison.Response{ status_code: 200, @@ -196,11 +192,7 @@ defmodule HTTPoisonBaseTest do test "having http_proxy env variable set on http requests" do System.put_env("HTTP_PROXY", "proxy") - expect(:hackney, :request, fn - :post, "http://localhost", [], "body", [proxy: "proxy"] -> {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_proxy("http://localhost", "proxy") assert HTTPoison.post!("localhost", "body") == %HTTPoison.Response{ @@ -214,11 +206,7 @@ defmodule HTTPoisonBaseTest do test "having http_proxy env variable set on http requests as empty string" do System.put_env("HTTP_PROXY", "") - expect(:hackney, :request, fn :post, "http://localhost", [], "body", [] -> - {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_no_proxy("http://localhost") assert HTTPoison.post!("localhost", "body") == %HTTPoison.Response{ @@ -232,11 +220,7 @@ defmodule HTTPoisonBaseTest do test "having https_proxy env variable set on https requests" do System.put_env("HTTPS_PROXY", "proxy") - expect(:hackney, :request, fn :post, "https://localhost", [], "body", [proxy: "proxy"] -> - {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_proxy("https://localhost", "proxy") assert HTTPoison.post!("https://localhost", "body") == %HTTPoison.Response{ @@ -250,11 +234,7 @@ defmodule HTTPoisonBaseTest do test "having https_proxy env variable set on http requests" do System.put_env("HTTPS_PROXY", "proxy") - expect(:hackney, :request, fn - :post, "http://localhost", [], "body", [] -> {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_no_proxy("http://localhost") assert HTTPoison.post!("localhost", "body") == %HTTPoison.Response{ @@ -269,15 +249,7 @@ defmodule HTTPoisonBaseTest do # If the variable is specified directly, no_proxy should be ignored. System.put_env("NO_PROXY", ".somedomain.com") - expect(:hackney, :request, fn :post, - "http://www.somedomain.com", - [], - "body", - [proxy: "proxy"] -> - {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_proxy("http://www.somedomain.com", "proxy") assert HTTPoison.post!("www.somedomain.com", "body", [], proxy: "proxy") == %HTTPoison.Response{ @@ -293,11 +265,7 @@ defmodule HTTPoisonBaseTest do System.put_env("HTTP_PROXY", "proxy") System.put_env("NO_PROXY", ".somedomain.com") - expect(:hackney, :request, fn :post, "http://www.somedomain.com", [], "body", [] -> - {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_no_proxy("http://www.somedomain.com") assert HTTPoison.post!("www.somedomain.com", "body") == %HTTPoison.Response{ @@ -312,11 +280,7 @@ defmodule HTTPoisonBaseTest do System.put_env("HTTP_PROXY", "proxy") System.put_env("NO_PROXY", ".nonmatching.com") - expect(:hackney, :request, fn :post, "http://www.somedomain.com", [], "body", [proxy: "proxy"] -> - {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_proxy("http://www.somedomain.com", "proxy") assert HTTPoison.post!("http://www.somedomain.com", "body") == %HTTPoison.Response{ @@ -331,11 +295,7 @@ defmodule HTTPoisonBaseTest do System.put_env("HTTP_PROXY", "proxy") System.put_env("NO_PROXY", ".nonmatching.com,.matching.com") - expect(:hackney, :request, fn :post, "http://www.matching.com", [], "body", [] -> - {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_no_proxy("http://www.matching.com") assert HTTPoison.post!("http://www.matching.com", "body") == %HTTPoison.Response{ @@ -350,11 +310,7 @@ defmodule HTTPoisonBaseTest do System.put_env("HTTP_PROXY", "proxy") System.put_env("NO_PROXY", ".nonmatching.com,*.matching.com") - expect(:hackney, :request, fn :post, "http://www.matching.com", [], "body", [] -> - {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_no_proxy("http://www.matching.com") assert HTTPoison.post!("http://www.matching.com", "body") == %HTTPoison.Response{ @@ -369,11 +325,7 @@ defmodule HTTPoisonBaseTest do System.put_env("HTTP_PROXY", "proxy") System.put_env("NO_PROXY", "*.nonmatching.com") - expect(:hackney, :request, fn :post, "http://www.matching.com", [], "body", [proxy: "proxy"] -> - {:ok, 200, "headers", :client} - end) - - expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + expect_hackney_post_with_proxy("http://www.matching.com", "proxy") assert HTTPoison.post!("http://www.matching.com", "body") == %HTTPoison.Response{ @@ -384,6 +336,22 @@ defmodule HTTPoisonBaseTest do } end + defp expect_hackney_post_with_proxy(url, proxy) do + expect_hackney_post(url, [proxy: proxy]) + end + + defp expect_hackney_post_with_no_proxy(url) do + expect_hackney_post(url, []) + end + + def expect_hackney_post(url, expected_options) do + expect(:hackney, :request, fn + :post, ^url, [], "body", ^expected_options -> {:ok, 200, "headers", :client} + end) + + expect(:hackney, :body, fn _, _ -> {:ok, "response"} end) + end + test "passing ssl option" do expect(:hackney, :request, fn :post, "http://localhost", From 7481d929628d699bf4ac7faf2f07a840bbfbd970 Mon Sep 17 00:00:00 2001 From: Shahne Rodgers Date: Mon, 8 Oct 2018 18:53:58 +1300 Subject: [PATCH 3/3] Formatting fixes. --- lib/httpoison/base.ex | 10 +++++----- test/httpoison_base_test.exs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/httpoison/base.ex b/lib/httpoison/base.ex index 3c4c0eb..0f3dfdf 100644 --- a/lib/httpoison/base.ex +++ b/lib/httpoison/base.ex @@ -622,9 +622,10 @@ defmodule HTTPoison.Base do defp check_no_proxy(proxy, request_url) do request_host = URI.parse(request_url).host - should_bypass_proxy = get_no_proxy_system_env() - |> String.split(",") - |> Enum.any?(fn domain -> matches_no_proxy_value?(request_host, domain) end) + should_bypass_proxy = + get_no_proxy_system_env() + |> String.split(",") + |> Enum.any?(fn domain -> matches_no_proxy_value?(request_host, domain) end) if should_bypass_proxy do nil @@ -634,8 +635,7 @@ defmodule HTTPoison.Base do end defp get_no_proxy_system_env() do - System.get_env("NO_PROXY") || System.get_env("no_PROXY") || System.get_env("no_proxy") || - "" + System.get_env("NO_PROXY") || System.get_env("no_PROXY") || System.get_env("no_proxy") || "" end defp matches_no_proxy_value?(request_host, no_proxy_value) do diff --git a/test/httpoison_base_test.exs b/test/httpoison_base_test.exs index 8da7936..755fb45 100644 --- a/test/httpoison_base_test.exs +++ b/test/httpoison_base_test.exs @@ -120,7 +120,7 @@ defmodule HTTPoisonBaseTest do test "passing proxy option" do expect_hackney_post_with_proxy("http://localhost", "proxy") - + assert HTTPoison.post!("localhost", "body", [], proxy: "proxy") == %HTTPoison.Response{ status_code: 200, @@ -337,7 +337,7 @@ defmodule HTTPoisonBaseTest do end defp expect_hackney_post_with_proxy(url, proxy) do - expect_hackney_post(url, [proxy: proxy]) + expect_hackney_post(url, proxy: proxy) end defp expect_hackney_post_with_no_proxy(url) do