Merge pull request #1047 from cerebris/includes_error_handling

Tightens the error handling on include parsing.

Author: dgeb

lib/jsonapi/include_directives.rb

@@ -52,7 +52,7 @@ def get_related(current_path)
           current_relationship = current_resource_klass._relationships[fragment]
           current_resource_klass = current_relationship.try(:resource_klass)
         else
-          warn "[RELATIONSHIP NOT FOUND] Relationship could not be found for #{current_path}."
+          raise JSONAPI::Exceptions::InvalidInclude.new(current_resource_klass, current_path)
         end
 
         include_in_join = @force_eager_load || !current_relationship || current_relationship.eager_load_on_include

lib/jsonapi/request_parser.rb

@@ -331,8 +331,7 @@ def check_include(resource_klass, include_parts)
                         include_parts.last.partition('.'))
         end
       else
-        @errors.concat(JSONAPI::Exceptions::InvalidInclude.new(format_key(resource_klass._type),
-                                                               include_parts.first).errors)
+        fail JSONAPI::Exceptions::InvalidInclude.new(format_key(resource_klass._type), include_parts.first)
       end
     end
 
@@ -352,12 +351,17 @@ def parse_include_directives(resource_klass, raw_include)
 
       return if included_resources.nil?
 
-      result = included_resources.compact.map do |included_resource|
-        check_include(resource_klass, included_resource.partition('.'))
-        unformat_key(included_resource).to_s
-      end
+      begin
+        result = included_resources.compact.map do |included_resource|
+          check_include(resource_klass, included_resource.partition('.'))
+          unformat_key(included_resource).to_s
+        end
 
-      JSONAPI::IncludeDirectives.new(resource_klass, result)
+        return JSONAPI::IncludeDirectives.new(resource_klass, result)
+      rescue JSONAPI::Exceptions::InvalidInclude => e
+        @errors.concat(e.errors)
+        return {}
+      end
     end
 
     def parse_filters(resource_klass, filters)

test/controllers/controller_test.rb

@@ -2043,7 +2043,6 @@ def test_expense_entries_show_bad_include_missing_relationship
     assert_cacheable_get :show, params: {id: 1, include: 'isoCurrencies,employees'}
     assert_response :bad_request
     assert_match /isoCurrencies is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
-    assert_match /employees is not a valid relationship of expenseEntries/, json_response['errors'][1]['detail']
   end
 
   def test_expense_entries_show_bad_include_missing_sub_relationship
@@ -2052,6 +2051,18 @@ def test_expense_entries_show_bad_include_missing_sub_relationship
     assert_match /post is not a valid relationship of people/, json_response['errors'][0]['detail']
   end
 
+  def test_invalid_include
+    assert_cacheable_get :index, params: {include: 'invalid../../../../'}
+    assert_response :bad_request
+    assert_match /invalid is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
+  end
+
+  def test_invalid_include_long_garbage_string
+    assert_cacheable_get :index, params: {include: 'invalid.foo.bar.dfsdfs,dfsdfs.sdfwe.ewrerw.erwrewrew'}
+    assert_response :bad_request
+    assert_match /invalid is not a valid relationship of expenseEntries/, json_response['errors'][0]['detail']
+  end
+
   def test_expense_entries_show_fields
     assert_cacheable_get :show, params: {id: 1, include: 'isoCurrency,employee', 'fields' => {'expenseEntries' => 'transactionDate'}}
     assert_response :success

test/unit/serializer/include_directives_test.rb

@@ -143,4 +143,22 @@ def test_three_levels_include_full_model_includes
     directives = JSONAPI::IncludeDirectives.new(PersonResource, ['posts.comments.tags'])
     assert_array_equals([{:posts=>[{:comments=>[:tags]}]}], directives.model_includes)
   end
+
+  def test_invalid_includes_1
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      JSONAPI::IncludeDirectives.new(PersonResource, ['../../../../']).include_directives
+    end
+  end
+
+  def test_invalid_includes_2
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      JSONAPI::IncludeDirectives.new(PersonResource, ['posts./sdaa./........']).include_directives
+    end
+  end
+
+  def test_invalid_includes_3
+    assert_raises JSONAPI::Exceptions::InvalidInclude do
+      JSONAPI::IncludeDirectives.new(PersonResource, ['invalid../../../../']).include_directives
+    end
+  end
 end