improve error telemetry for insecure URIs

refs FOO-969

TEST PLAN:
  1) Check an invalid URL through insecure_host
  2) it should return a specific UnresolvableUriError
     instead of calling it insecure
  3) error message should contain the offending host

Change-Id: I2a27973a5ed9f38ea8c79eb9c1be5c8ee78e2d76
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249525
Reviewed-by: Ahmad Amireh <ahmad@instructure.com>
QA-Review: Ahmad Amireh <ahmad@instructure.com>
Product-Review: Ahmad Amireh <ahmad@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>

gems/canvas_http/lib/canvas_http.rb

@@ -40,6 +40,7 @@ module CanvasHttp
   end
   class RelativeUriError < CanvasHttp::Error; end
   class InsecureUriError < CanvasHttp::Error; end
+  class UnresolvableUriError < CanvasHttp::Error; end
   class CircuitBreakerError < CanvasHttp::Error; end
 
   def self.put(*args, &block)
@@ -180,13 +181,34 @@ module CanvasHttp
 
   def self.insecure_host?(host)
     return unless filters = self.blocked_ip_filters
-    addrs = Resolv.getaddresses(host).map { |ip| ::IPAddr.new(ip) rescue nil}.compact
-    return true unless addrs.any?
-
-    filters.any? do |filter|
-      addr_range = ::IPAddr.new(filter) rescue nil
-      addr_range && addrs.any?{|addr| addr_range.include?(addr)}
+    resolved_addrs = Resolv.getaddresses(host)
+    unless resolved_addrs.any?
+      # this is actually a different condition than the host being insecure,
+      # and having separate telemetry is helpful for understanding transient failures.
+      raise UnresolvableUriError, "#{host} cannot be resolved to any address"
     end
+    ip_addrs = resolved_addrs.map do |ip|
+      ::IPAddr.new(ip)
+    rescue IPAddr::InvalidAddressError
+      # this should never happen, Resolv should only be passing back IPs, but
+      # let's make sure we can see if the impossible occurs
+      logger.warn("CANVAS_HTTP WARNING | host: #{host} | invalid_ip: #{ip}")
+      nil
+    end.compact
+    unless ip_addrs.any?
+      raise UnresolvableUriError, "#{host} resolves to only unparseable IPs..."
+    end
+
+    filters.each do |filter|
+      addr_range = ::IPAddr.new(filter)
+      ip_addrs.any? do |addr|
+        if addr_range.include?(addr)
+          logger.warn("CANVAS_HTTP WARNING insecure address | host: #{host} | insecure_address: #{addr} | filter: #{filter}")
+          return true
+        end
+      end
+    end
+    false
   end
 
   # returns a Net::HTTP connection object for the given URI object

gems/canvas_http/spec/canvas_http_spec.rb

@@ -209,19 +209,32 @@ describe "CanvasHttp" do
   end
 
   describe '#insecure_host?' do
+    around(:each) do |example|
+      old_filters = CanvasHttp.blocked_ip_filters
+      CanvasHttp.blocked_ip_filters = -> { ['127.0.0.1/8', '42.42.42.42/16']}
+      example.call
+    ensure
+      CanvasHttp.blocked_ip_filters = old_filters
+    end
+
     it "should check for insecure hosts" do
-      begin
-        old_filters = CanvasHttp.blocked_ip_filters
-        CanvasHttp.blocked_ip_filters = -> { ['127.0.0.1/8', '42.42.42.42/16']}
-        expect(CanvasHttp.insecure_host?('example.com')).to eq false
-        expect(CanvasHttp.insecure_host?('localhost')).to eq true
-        expect(CanvasHttp.insecure_host?('127.0.0.1')).to eq true
-        expect(CanvasHttp.insecure_host?('42.42.42.42')).to eq true
-        expect(CanvasHttp.insecure_host?('42.42.1.1')).to eq true
-        expect(CanvasHttp.insecure_host?('42.1.1.1')).to eq false
-      ensure
-        CanvasHttp.blocked_ip_filters = old_filters
-      end
+      expect(CanvasHttp.insecure_host?('example.com')).to eq false
+      expect(CanvasHttp.insecure_host?('localhost')).to eq true
+      expect(CanvasHttp.insecure_host?('127.0.0.1')).to eq true
+      expect(CanvasHttp.insecure_host?('42.42.42.42')).to eq true
+      expect(CanvasHttp.insecure_host?('42.42.1.1')).to eq true
+      expect(CanvasHttp.insecure_host?('42.1.1.1')).to eq false
+    end
+
+    it "raises an error when URL is not resolveable" do
+      bad_url = 'this-should-never-be-a-real-url-registered-by-anyone.fake-tld'
+      expect{ CanvasHttp.insecure_host?(bad_url) }.to raise_error(CanvasHttp::UnresolvableUriError)
+    end
+
+    it "won't continue to process a host with no valid IPs" do
+      bad_url = 'this-should-never-be-a-real-url-registered-by-anyone.fake-tld'
+      expect(Resolv).to receive(:getaddresses).with(bad_url).and_return(["not.an.ip.address"])
+      expect{ CanvasHttp.insecure_host?(bad_url) }.to raise_error(CanvasHttp::UnresolvableUriError)
     end
   end