slab / safeurl-elixir Goto Github PK

Resolving DNS in test environment may make tests flaky. An example from CI build:

** (Socket.Error) timeout
    (socket 0.3.13) lib/socket/datagram.ex:46: Socket.Datagram.recv!/2
    (dns 2.3.0) lib/dns.ex:76: DNS.query/4
    (dns 2.3.0) lib/dns.ex:23: DNS.resolve/4
    (safeurl 0.2.0) lib/safeurl.ex:267: SafeURL.resolve_address/1
    (safeurl 0.2.0) lib/safeurl.ex:136: SafeURL.allowed?/2
    (safeurl 0.2.0) lib/safeurl.ex:177: SafeURL.validate/2

There are various HTTP clients in Elixir-land, and devs prefer using one over the other. SafeURL should avoid importing an entire HTTP client as a dependency just to make a GET request, and just let the user choose what to call.

After #1, users can simply call something such as:

with :ok <- SafeURL.validate(url) do
  HTTPClientOfMyChoice.get(url)
end

At the very least, it should use the built-in :httpc module or mark :httpoison as an optional dependency in mix.exs so it's not loaded unless explicitly specified by the user.

unit tests broke after 1ac2e81

  1) test #allowed? only allows whitelist when present (SafeURLTest)
     test/safeurl_test.exs:38
     Expected truthy, got false
     code: assert SafeURL.allowed?("http://10.0.0.1/", opts)
     arguments:

         # 1
         "http://10.0.0.1/"

         # 2
         [whitelist: ["10.0.0.0/24"]]

     stacktrace:
       test/safeurl_test.exs:41: (test)



  2) test #allowed? allows blacklisting custom IP ranges (SafeURLTest)
     test/safeurl_test.exs:29
     Expected false or nil, got true
     code: refute SafeURL.allowed?("http://5.5.5.5", opts)
     arguments:

         # 1
         "http://5.5.5.5"

         # 2
         [blacklist: ["5.5.0.0/16", "100.0.0.0/24"]]

     stacktrace:
       test/safeurl_test.exs:34: (test)

.

Finished in 1.2 seconds (0.00s async, 1.2s sync)
4 tests, 2 failures

Hi,

Thanks for this library. Would it be possible to bump the lock file to accept HTTPoison 2.x please?

Example and get() are likely unsafe because the host is resolved twice. Once by checking and once by the http client. It is possible for the DNS to resolve differently which would bypass the checking.