Merge pull request #6890 from SuperTux88/connection-tester-refactoring

Improve ConnectionTester

lib/connection_tester.rb

@@ -1,7 +1,9 @@
 
 class ConnectionTester
-  NODEINFO_SCHEMA   = "http://nodeinfo.diaspora.software/ns/schema/1.0"
+  include Diaspora::Logging
-  NODEINFO_FRAGMENT = "/.well-known/nodeinfo"
+
+  NODEINFO_SCHEMA   = "http://nodeinfo.diaspora.software/ns/schema/1.0".freeze
+  NODEINFO_FRAGMENT = "/.well-known/nodeinfo".freeze
 
   class << self
     # Test the reachability of a server by the given HTTP/S URL.
@@ -74,7 +76,7 @@ class ConnectionTester
   rescue URI::InvalidURIError => e
     raise AddressFailure, e.message
   rescue StandardError => e
-    raise Failure, e.inspect
+    unexpected_error(e)
   end
 
   # Perform the DNS query, the IP address will be stored in the result
@@ -84,7 +86,7 @@ class ConnectionTester
   rescue SocketError => e
     raise DNSFailure, "'#{@uri.host}' - #{e.message}"
   rescue StandardError => e
-    raise Failure, e.inspect
+    unexpected_error(e)
   end
 
   # Perform a HTTP GET request to determine the following information
@@ -113,7 +115,7 @@ class ConnectionTester
   rescue ArgumentError, FaradayMiddleware::RedirectLimitReached, Faraday::ClientError => e
     raise HTTPFailure, e.message
   rescue StandardError => e
-    raise Failure, e.inspect
+    unexpected_error(e)
   end
 
   # Try to find out the version of the other servers software.
@@ -127,10 +129,14 @@ class ConnectionTester
       nd_resp = http.get(find_nodeinfo_url(ni_resp.body))
       find_software_version(nd_resp.body)
     end
+  rescue NodeInfoFailure => e
+    raise e
+  rescue JSON::Schema::ValidationError, JSON::Schema::SchemaError => e
+    raise NodeInfoFailure, "#{e.class}: #{e.message}"
   rescue Faraday::ResourceNotFound, JSON::JSONError => e
     raise NodeInfoFailure, e.message[0..255].encode(Encoding.default_external, undef: :replace)
   rescue StandardError => e
-    raise Failure, e.inspect
+    unexpected_error(e)
   end
 
   private
@@ -179,8 +185,10 @@ class ConnectionTester
 
   # walk the JSON document, get the actual document location
   def find_nodeinfo_url(body)
-    links = JSON.parse(body)
+    jrd = JSON.parse(body)
-    links.fetch("links").find { |entry|
+    links = jrd.fetch("links")
+    raise NodeInfoFailure, "invalid JRD: '#/links' is not an array!" unless links.is_a?(Array)
+    links.find { |entry|
       entry.fetch("rel") == NODEINFO_SCHEMA
     }.fetch("href")
   end
@@ -188,10 +196,16 @@ class ConnectionTester
   # walk the JSON document, find the version string
   def find_software_version(body)
     info = JSON.parse(body)
+    JSON::Validator.validate!(NodeInfo.schema("1.0"), info)
     sw = info.fetch("software")
     @result.software_version = "#{sw.fetch('name')} #{sw.fetch('version')}"
   end
 
+  def unexpected_error(error)
+    logger.error "unexpected error: #{error.class}: #{error.message}\n#{error.backtrace.first(15).join("\n")}"
+    raise Failure, error.inspect
+  end
+
   class Failure < StandardError
   end
 

spec/lib/connection_tester_spec.rb

@@ -113,13 +113,21 @@ describe ConnectionTester do
   end
 
   describe "#nodeinfo" do
+    let(:ni_wellknown) { {links: [{rel: ConnectionTester::NODEINFO_SCHEMA, href: "/nodeinfo"}]} }
+
     it "reads the version from the nodeinfo document" do
-      ni_wellknown = {links: [{rel: ConnectionTester::NODEINFO_SCHEMA, href: "/nodeinfo"}]}
+      ni_document = NodeInfo.build do |doc|
-      ni_document = {software: {name: "diaspora", version: "a.b.c.d"}}
+        doc.version = "1.0"
+        doc.open_registrations = true
+        doc.protocols.inbound << "diaspora"
+        doc.protocols.outbound << "diaspora"
+        doc.software.name = "diaspora"
+        doc.software.version = "a.b.c.d"
+      end
 
       stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}")
         .to_return(status: 200, body: JSON.generate(ni_wellknown))
-      stub_request(:get, "#{url}/nodeinfo").to_return(status: 200, body: JSON.generate(ni_document))
+      stub_request(:get, "#{url}/nodeinfo").to_return(status: 200, body: JSON.generate(ni_document.as_json))
 
       tester.nodeinfo
       expect(result.software_version).to eq("diaspora a.b.c.d")
@@ -136,5 +144,19 @@ describe ConnectionTester do
         .to_return(status: 200, body: '{"json"::::"malformed"}')
       expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure)
     end
+
+    it "handles a invalid jrd document gracefully" do
+      invalid_wellknown = {links: {rel: ConnectionTester::NODEINFO_SCHEMA, href: "/nodeinfo"}}
+      stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}")
+        .to_return(status: 200, body: JSON.generate(invalid_wellknown))
+      expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure)
+    end
+
+    it "handles a invalid nodeinfo document gracefully" do
+      stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}")
+        .to_return(status: 200, body: JSON.generate(ni_wellknown))
+      stub_request(:get, "#{url}/nodeinfo").to_return(status: 200, body: '{"software": "invalid nodeinfo"}')
+      expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure)
+    end
   end
 end