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

Faraday does not parse URLs correctly #428

Closed
comandeo opened this issue Oct 26, 2014 · 4 comments · Fixed by #531
Closed

Faraday does not parse URLs correctly #428

comandeo opened this issue Oct 26, 2014 · 4 comments · Fixed by #531

Comments

@comandeo
Copy link

Code example below:

# Correct behaviour (Net::HTTP)
uri = URI.parse('http://www.webservicex.net/CurrencyConvertor.asmx?WSDL')
response = Net::HTTP.get_response(uri)
puts response.code # 200
puts response.body # WSDL content

# Incorrect behaviour
connection = Faraday.new('http://www.webservicex.net')
faraday_response = connection.get('/CurrencyConvertor.asmx?WSDL')
puts faraday_response.status # 500
puts faraday_response.body # Error message

After some investigation I found that URL http://www.webservicex.net/CurrencyConvertor.asmx?WSDL is being converted into URL:http://www.webservicex.net/CurrencyConvertor.asmx?WSDL= with and additional symbol = at the end.

puts faraday_response.env.url # http://www.webservicex.net/CurrencyConvertor.asmx?WSDL=

Seems that Faraday::NestedParamsEncoder.decode method considers query string ?WSDL as a parameter with empty value.

uri = URI.parse('http://www.webservicex.net/CurrencyConvertor.asmx?WSDL')
decoded_query = Faraday::NestedParamsEncoder.decode(uri.query) # =>  {"WSDL"=>nil}
Faraday::NestedParamsEncoder.encode(decoded_query) # => "WSDL="

Since URLs of such kind a quite typical for WSDLs of web services, it would be great if we could find a solution for this issue.

@rusikf
Copy link

rusikf commented Nov 13, 2014

@comandeo - Does WSDL service url can have multiple empty parameters like this:

http://www.webservicex.net/CurrencyConvertor.asmx?WSDL&OTHER_PARAM

or empty param can be only last param?( In your example - WSDL)
Encoding "a" to {a => nil} and decoding works nice for usual urls

@comandeo
Copy link
Author

Though usually WSDL urls do not have multiple empty params, your example is still a valid url according to RFC 3986. URL part after ? does not have to be in key=value form.

@comandeo comandeo reopened this Nov 25, 2014
@rusikf
Copy link

rusikf commented Nov 28, 2014

@comandeo , it looks that those urls are valid:
rubygems.org/about.html?a=
rubygems.org/about.html?a
rubygems.org/about.html?a=&b
I noticed, that in parameters_test.rb there is a code :

 def test_encode_nil_nested
    assert_equal "a=", Faraday::NestedParamsEncoder.encode("a" => nil)
 end

So it, looks, that

uri = URI.parse('http://www.webservicex.net/CurrencyConvertor.asmx?WSDL')
decoded_query = Faraday::NestedParamsEncoder.decode(uri.query) # =>  {"WSDL"=>nil}
Faraday::NestedParamsEncoder.encode(decoded_query) # => "WSDL="

works as a maintainer's predictable behaviour
May be it will be nice to add some additional option like this

  connection.get('/CurrencyConvertor.asmx?WSDL') do |req|
     req.options.compact = true
  end

What do you think ? I wish, some of the main contributors help us about ideas how it must work( @mislav, @technoweenie , @sferik, @zenhob or etc )

@PadraigK
Copy link

Adding an = should certainly be considered a bug. Here is an example URL where the additional = causes an undesirable redirect: http://jayu.universpodcast.com/feed/?lang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants