Merge pull request #931 from ElixirTeSS/avoid-parsing

Don't attempt to parse content when testing URLs
ElixirTeSS · Mar 5, 2024 · 55b5933 · 55b5933
2 parents 6926a8f + 959a218
commit 55b5933
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 5 deletions.
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -60,8 +60,8 @@ def test_url
       uri = URI.parse(params[:url]) rescue nil
       if uri && (uri.scheme == 'http' || uri.scheme == 'https')
         PrivateAddressCheck.only_public_connections do
-          res = HTTParty.get(uri.to_s, { timeout: 5 })
-          body = { code: res.code, message: res.message }
+          res = HTTParty.get(uri.to_s, timeout: 5, format: :plain)
+          body = { code: res.code, message: 'OK' }
         end
       else
         body = { message: 'Invalid URL - Make sure the URL starts with "https://" or "http://"' }

diff --git a/app/controllers/bioschemas_controller.rb b/app/controllers/bioschemas_controller.rb
@@ -39,7 +39,7 @@ def fetch_url
       uri = URI.parse(params[:url]) rescue nil
       if uri && (uri.scheme == 'http' || uri.scheme == 'https')
         PrivateAddressCheck.only_public_connections do
-          res = HTTParty.get(uri.to_s, { timeout: 5 })
+          res = HTTParty.get(uri.to_s, timeout: 5, format: :plain)
           if res.code == 200
             res.body
           else

diff --git a/test/controllers/application_controller_test.rb b/test/controllers/application_controller_test.rb
@@ -10,6 +10,9 @@ class ApplicationControllerTest < ActionController::TestCase
     WebMock.stub_request(:any, 'http://500host.com').to_return(status: 500, body: 'hi')
     WebMock.stub_request(:any, 'http://slowhost.com').to_timeout
     WebMock.stub_request(:any, 'http://notrealhost.goldfish').to_raise(SocketError)
+    WebMock.stub_request(:any, 'http://badformathost.com').to_return(status: 200,
+                                                                     body: '<h1>Not really JSON</h1>',
+                                                                     headers: { content_type: 'application/json' })
 
     sign_in users(:regular_user)
   end
@@ -24,6 +27,9 @@ class ApplicationControllerTest < ActionController::TestCase
     get :test_url, params: { url: 'http://500host.com', format: :json }
     assert_equal 500, JSON.parse(response.body)['code']
 
+    get :test_url, params: { url: 'http://badformathost.com', format: :json }
+    assert_equal 200, JSON.parse(response.body)['code']
+
     get :test_url, params: { url: 'http://slowhost.com', format: :json }
     assert_equal 'Could not access the given URL', JSON.parse(response.body)['message']
 

diff --git a/test/controllers/bioschemas_controller_test.rb b/test/controllers/bioschemas_controller_test.rb
@@ -55,13 +55,12 @@ class BioschemasControllerTest < ActionController::TestCase
 
   test 'should test JSON-LD URL' do
     WebMock.stub_request(:get, 'https://website.com/material.json').
-      to_return(status: 200, headers: {}, body: fixture_file('ext_res.json').read)
+      to_return(status: 200, headers: { content_type: 'application/json' }, body: fixture_file('ext_res.json').read)
 
     sign_in users(:regular_user)
 
     post :run_test, params: { url: 'https://website.com/material.json' }
 
-
     output = assigns(:output)
     assert_equal 1, output[:totals]['LearningResources']
     assert_equal 1, output[:resources][:materials].count
@@ -99,6 +98,25 @@ class BioschemasControllerTest < ActionController::TestCase
     JSON::LD::Reader.define_method(old_method.name, old_method)
   end
 
+  test 'should gracefully handle malformed JSON-LD URL' do
+    # Silence error stdout from RDF library
+    old_method = JSON::LD::Reader.instance_method(:logger_common)
+    JSON::LD::Reader.define_method(:logger_common) { |*args| }
+
+    WebMock.stub_request(:get, 'https://website.com/material.json').
+      to_return(status: 200, body: '{ { "wut ;}',
+                headers: { content_type: 'application/json' })
+
+    sign_in users(:regular_user)
+
+    post :run_test, params: { url: 'https://website.com/material.json' }
+
+    assert_response :unprocessable_entity
+    assert flash[:error].include?('parsing error')
+  ensure
+    JSON::LD::Reader.define_method(old_method.name, old_method)
+  end
+
   test 'should gracefully handle URL timeout' do
     WebMock.stub_request(:get, 'https://website.com/material.html').to_timeout
     sign_in users(:regular_user)