Skip to content

Commit

Permalink
fix redirect uri format in Client#request
Browse files Browse the repository at this point in the history
  • Loading branch information
@SophieDeBenedetto
SophieDeBenedetto committed Jul 19, 2016
1 parent fb502c7 commit bf38948
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions lib/oauth2/client.rb
View file
Expand Up @@ -89,8 +89,14 @@ def request(verb, url, opts = {}) # rubocop:disable CyclomaticComplexity, Method
connection.response :logger, ::Logger.new($stdout) if ENV['OAUTH_DEBUG'] == 'true' connection.response :logger, ::Logger.new($stdout) if ENV['OAUTH_DEBUG'] == 'true'


url = connection.build_url(url, opts[:params]).to_s url = connection.build_url(url, opts[:params]).to_s

body = nil

This comment has been minimized.

Sign in to view

Copy link
@aviflombaum

aviflombaum Aug 2, 2016 •
edited

Member

I think this is slightly clearn for LOC 92-98

body = if opts[:body]      
  opts[:body][:redirect_uri] = opts[:body][:redirect_uri].split("?").first
  URI.encode_www_form(opts[:body])
else
  opts
end

I'm also wondering if you really need to re-write opts[:body][:redirect_uri] and can rather leave the original value alone and do something like:

body = opts[:body] ? URI.encode_www_form(opts[:body][:redirect_uri].split("?").first) : opts

I would also suggest naming what you're doing with opts[:body][:redirect_uri].split("?").first it's some sort of sanitation and a private method seems like a good home so that you get something like:

body = opts[:body] ? URI.encode_www_form(sanitize_querystring_on_body_rediect_uri) : opts
response = connection.run_request(verb, url, opts[:body], opts[:headers]) do |req| if opts[:body]
opts[:body][:redirect_uri] = opts[:body][:redirect_uri].split("?").first
body = URI.encode_www_form(opts[:body])
else
body = opts
end
response = connection.run_request(verb, url, body, opts[:headers]) do |req|
yield(req) if block_given? yield(req) if block_given?
end end
response = Response.new(response, :parse => opts[:parse]) response = Response.new(response, :parse => opts[:parse])
Expand Down

0 comments on commit bf38948

Please sign in to comment.