Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compare URI-decoded path params #482

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/graphiti.rb
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ def self.cache
require "graphiti/query"
require "graphiti/debugger"
require "graphiti/util/cache_debug"
require "graphiti/util/uri_decoder"

if defined?(ActiveRecord)
require "graphiti/adapters/active_record"
Expand Down
8 changes: 8 additions & 0 deletions lib/graphiti/adapters/null.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ def filter_boolean_eq(scope, attribute, value)
scope
end

def filter_uuid_eq(scope, attribute, value)
scope
end

def filter_uuid_not_eq(scope, attribute, value)
scope
end

def base_scope(model)
{}
end
Expand Down
30 changes: 30 additions & 0 deletions lib/graphiti/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class Configuration
attr_accessor :before_sideload

attr_reader :debug, :debug_models
attr_reader :uri_decoder

attr_writer :schema_path
attr_writer :cache_rendering
Expand All @@ -37,6 +38,8 @@ def initialize
self.debug = ENV.fetch("GRAPHITI_DEBUG", true)
self.debug_models = ENV.fetch("GRAPHITI_DEBUG_MODELS", false)

@uri_decoder = infer_uri_decoder

# FIXME: Don't duplicate graphiti-rails efforts
if defined?(::Rails.root) && (root = ::Rails.root)
config_file = root.join(".graphiticfg.yml")
Expand Down Expand Up @@ -85,6 +88,33 @@ def with_option(key, value)
ensure
send(:"#{key}=", original)
end

def uri_decoder=(decoder)
unless decoder.respond_to?(:call)
raise "uri_decoder must respond to `call`."
end

@uri_decoder = decoder
end

private

def infer_uri_decoder
if defined?(::ActionDispatch::Journey::Router::Utils)
# available in all supported versions of Rails.
# This method should be preferred for comparing URI path segments
# to params, as it is the exact decoder used in the Rails router.
@uri_decoder = ::ActionDispatch::Journey::Router::Utils.method(:unescape_uri)
elsif URI.respond_to?(:decode_uri_component)
# available in Ruby >= 3.2
@uri_decoder = URI.method(:decode_uri_component)
end
rescue => e
Kernel.warn("Error inferring Graphiti uri_decoder: #{e}")
ensure
# fallback
@uri_decoder ||= :itself.to_proc
end
end

msg = "Use graphiti-rails's `config.graphiti.respond_to_formats`"
Expand Down
6 changes: 3 additions & 3 deletions lib/graphiti/resource/links.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ def allow_request?(request_path, params, action)
endpoints.any? do |e|
has_id = params[:id] || params[:data].try(:[], :id)
path = request_path
if [:update, :show, :destroy].include?(context_namespace) && has_id
if [:update, :show, :destroy].include?(action) && has_id
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

action is passed in by the caller; seems appropriate to use it here instead of duplicating the call to context_namespace

path = request_path.split("/")
path.pop if path.last == has_id.to_s
path.pop if Graphiti::Util::UriDecoder.decode_uri(path.last) == has_id.to_s
path = path.join("/")
end
e[:full_path].to_s == path && e[:actions].include?(context_namespace)
e[:full_path].to_s == path && e[:actions].include?(action)
end
end

Expand Down
9 changes: 9 additions & 0 deletions lib/graphiti/util/uri_decoder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Graphiti
module Util
module UriDecoder
def self.decode_uri(uri)
Graphiti.config.uri_decoder.call(uri)
end
end
end
end
17 changes: 17 additions & 0 deletions spec/resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,23 @@ def self.name
end
end
end

context "with a url-encoded path param" do
before do
request.env["PATH_INFO"] += "/123%3B456"

klass.instance_exec do
attribute :id, :uuid
end
end

it "works" do
klass
Graphiti.with_context ctx, :show do
expect { klass.find(id: "123;456") }.to_not raise_error
end
end
end
end

context "and the request matches a secondary endpoint" do
Expand Down
10 changes: 10 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,13 @@ def check!
database: ":memory:"
Dir[File.dirname(__FILE__) + "/fixtures/**/*.rb"].sort.each { |f| require f }
end

if Gem::Version.new(RUBY_VERSION) < Gem::Version.new("3.2.0")
unless defined?(::ActionDispatch::Journey)
require 'uri'
# NOTE: `decode_www_form_component` isn't an ideal default for production,
# because it varies slightly compared to typical uri parameterization,
# but it will allow tests to pass in non-rails contexts.
Graphiti.config.uri_decoder = URI.method(:decode_www_form_component)
end
end
Loading