From 03c6fcdf6f8fcecf98caa413897cea83e7802b7e Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 19 Feb 2015 10:31:13 +0100 Subject: [PATCH] rewrite ResourceLinkParser - use own RegExp instead of borrowing the ones from Grape routes - add specs --- lib/api/utilities/resource_link_parser.rb | 32 +++--- lib/api/v3/render/render_api.rb | 5 +- lib/api/v3/utilities/custom_field_injector.rb | 5 +- ...ork_package_attribute_links_representer.rb | 5 +- .../work_packages/link_to_object_extractor.rb | 2 +- .../utilities/resource_link_parser_spec.rb | 100 ++++++++++++++++++ spec/requests/api/v3/render_resource_spec.rb | 8 +- 7 files changed, 132 insertions(+), 25 deletions(-) create mode 100644 spec/lib/api/utilities/resource_link_parser_spec.rb diff --git a/lib/api/utilities/resource_link_parser.rb b/lib/api/utilities/resource_link_parser.rb index 60628e31a6..b007574d04 100644 --- a/lib/api/utilities/resource_link_parser.rb +++ b/lib/api/utilities/resource_link_parser.rb @@ -30,27 +30,25 @@ module API module Utilities module ResourceLinkParser - def self.parse(resource_link) - ::API::Root.routes.each do |route| - route_options = route.instance_variable_get(:@options) + # N.B. valid characters for URL path segments as of + # http://tools.ietf.org/html/rfc3986#section-3.3 + SEGMENT_CHARACTER = '(\w|[-~!$&\'\(\)*+\.,:;=@]|%[0-9A-Fa-f]{2})' + RESOURCE_REGEX = "/api/v(?\\d)/(?\\w+)/(?#{SEGMENT_CHARACTER}*)\\z" - # we are matching the resource link without trailing slashes, as they would still make - # for a valid link (even though it does not match the compiled regex) - match = route_options[:compiled].match(resource_link.chomp('/')) + def self.resource_matcher + @@matcher ||= Regexp.compile(RESOURCE_REGEX) + end - if match - # we want to capture the identifying key regardless of its name (e.g. :id) - id_key = match.names.reject { |name| ['version', 'format'].include?(name) }.first - id = id_key ? match[id_key] : nil + def self.parse(resource_link) + match = resource_matcher.match(resource_link) - return { - ns: /\/(?\w+)/.match(route_options[:namespace])[:ns], - id: id - } - end - end + return nil unless match - nil + return { + version: match[:version], + namespace: match[:namespace], + id: match[:id] + } end end end diff --git a/lib/api/v3/render/render_api.rb b/lib/api/v3/render/render_api.rb index 96081b1d1d..f873b283f8 100644 --- a/lib/api/v3/render/render_api.rb +++ b/lib/api/v3/render/render_api.rb @@ -81,7 +81,7 @@ module API if params[:context] context = parse_context - case context[:ns] + case context[:namespace] when 'work_packages' WorkPackage.visible(current_user).find(context[:id]) end @@ -95,7 +95,8 @@ module API fail API::Errors::InvalidRenderContext.new( I18n.t('api_v3.errors.render.context_not_found') ) - elsif !SUPPORTED_CONTEXT_NAMESPACES.include? context[:ns] + elsif !SUPPORTED_CONTEXT_NAMESPACES.include?(context[:namespace]) || + context[:version] != '3' fail API::Errors::InvalidRenderContext.new( I18n.t('api_v3.errors.render.unsupported_context') ) diff --git a/lib/api/v3/utilities/custom_field_injector.rb b/lib/api/v3/utilities/custom_field_injector.rb index 89505a3e49..ad14211789 100644 --- a/lib/api/v3/utilities/custom_field_injector.rb +++ b/lib/api/v3/utilities/custom_field_injector.rb @@ -114,8 +114,9 @@ module API resource = ::API::Utilities::ResourceLinkParser.parse href - if resource.nil? || resource[:ns] != expected_namespace - actual_namespace = resource ? resource[:ns] : nil + if resource.nil? || resource[:namespace] != expected_namespace || + resource[:version] != '3' + actual_namespace = resource ? resource[:namespace] : nil fail ::API::Errors::Form::InvalidResourceLink.new(property, expected_namespace, diff --git a/lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb b/lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb index f9f3963bc2..05bc708964 100644 --- a/lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb +++ b/lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb @@ -127,8 +127,9 @@ module API resource = ::API::Utilities::ResourceLinkParser.parse href - if resource.nil? || resource[:ns] != ns.to_s - actual_ns = resource ? resource[:ns] : nil + if resource.nil? || resource[:namespace] != ns.to_s || + resource[:version] != '3' + actual_ns = resource ? resource[:namespace] : nil property_localized = I18n.t("attributes.#{property}") expected_localized = I18n.t("attributes.#{ns.to_s.singularize}") diff --git a/lib/api/v3/work_packages/link_to_object_extractor.rb b/lib/api/v3/work_packages/link_to_object_extractor.rb index 7289d7aa34..f6db452cb2 100644 --- a/lib/api/v3/work_packages/link_to_object_extractor.rb +++ b/lib/api/v3/work_packages/link_to_object_extractor.rb @@ -36,7 +36,7 @@ module API resource = ::API::Utilities::ResourceLinkParser.parse links[attribute]['href'] if resource - case resource[:ns] + case resource[:namespace] when 'statuses' h[:status_id] = resource[:id] end diff --git a/spec/lib/api/utilities/resource_link_parser_spec.rb b/spec/lib/api/utilities/resource_link_parser_spec.rb new file mode 100644 index 0000000000..960a72003d --- /dev/null +++ b/spec/lib/api/utilities/resource_link_parser_spec.rb @@ -0,0 +1,100 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +describe ::API::Utilities::ResourceLinkParser do + subject { described_class } + + describe('#parse') do + shared_examples_for 'accepts resource link' do + it 'parses the version' do + expect(result[:version]).to eql(version) + end + + it 'parses the namespace' do + expect(result[:namespace]).to eql(namespace) + end + + it 'parses the id' do + expect(result[:id]).to eql(id) + end + end + + shared_examples_for 'rejects resource link' do + it 'is nil' do + expect(result).to be_nil + end + end + + describe 'accepts a simple resource' do + it_behaves_like 'accepts resource link' do + let(:result) { subject.parse '/api/v3/statuses/12' } + let(:version) { '3' } + let(:namespace) { 'statuses' } + let(:id) { '12' } + end + end + + describe 'accepts all valid segment characters as id' do + it_behaves_like 'accepts resource link' do + let(:result) { subject.parse '/api/v3/string_object/foo-2_~!$&\'()*+.,:;=@%Fa' } + let(:version) { '3' } + let(:namespace) { 'string_object' } + let(:id) { 'foo-2_~!$&\'()*+.,:;=@%Fa' } + end + end + + describe 'accepts resource with empty id segment' do + it_behaves_like 'accepts resource link' do + let(:result) { subject.parse '/api/v3/string_object/' } + let(:version) { '3' } + let(:namespace) { 'string_object' } + let(:id) { '' } + end + end + + describe 'rejects resource with missing id segment' do + it_behaves_like 'rejects resource link' do + let(:result) { subject.parse '/api/v3/string_object' } + end + end + + describe 'rejects the api root' do + it_behaves_like 'rejects resource link' do + let(:result) { subject.parse '/api/v3/' } + end + end + + describe 'rejects nested resources' do + it_behaves_like 'rejects resource link' do + let(:result) { subject.parse '/api/v3/statuses/imaginary/' } + end + end + end +end diff --git a/spec/requests/api/v3/render_resource_spec.rb b/spec/requests/api/v3/render_resource_spec.rb index 3049799f4b..d9847e8b51 100644 --- a/spec/requests/api/v3/render_resource_spec.rb +++ b/spec/requests/api/v3/render_resource_spec.rb @@ -102,11 +102,17 @@ describe 'API v3 Render resource' do I18n.t('api_v3.errors.render.context_not_found') end - describe 'unsupported context found' do + describe 'unsupported context resource found' do let(:post_path) { "#{path}?context=/api/v3/activities/2" } it_behaves_like 'invalid render context', 'Unsupported context found.' end + + describe 'unsupported context version found' do + let(:post_path) { "#{path}?context=/api/v4/work_packages/2" } + + it_behaves_like 'invalid render context', 'Unsupported context found.' + end end end end