Encoding::UndefinedConversionError while parsing responses. #270
Description
Asana returns extra data in the get_token reponse pertaining to the current user (which MAY have unicode UTF-8 data). Because the underlying network layer (ruby's Net::HTTP) returns data as ASCII-8BIT, the current code causes this error to occur.
Note this is a SUCCESSFUL token reponse (e.g. no errors)
Backtrace
/gems/oauth2-1.2.0/lib/oauth2/error.rb:30 in encode
/gems/oauth2-1.2.0/lib/oauth2/error.rb:30 in error_message
/gems/oauth2-1.2.0/lib/oauth2/error.rb:17 in initialize
/gems/oauth2-1.2.0/lib/oauth2/client.rb:139 in new
/gems/oauth2-1.2.0/lib/oauth2/client.rb:139 in get_token
/gems/oauth2-1.2.0/lib/oauth2/strategy/auth_code.rb:29 in get_token
A few questions.
- Why is an error instance always created on line 139 of client.rb if the check for having an error in the first place happens on line 140? It would make better sense to create that instance IN the raise(error) instead?
- why is the error message routine changing the encoding of the response body anyways? and why to the "hand-built" string up on line 14?
- which is always US-ASCII in this scenario as code and description are both nil.
- "#{nil}: #{nil}".encoding == US-ASCII
- outputting a nil in that above string cause the encoding to degenerate to US-ASCII, even if one is UTF-8 or ASCII-8BIT
- which is always US-ASCII in this scenario as code and description are both nil.
However, regardless, even if the error string data WERE correct, if there is ANY UTF-8 data in the string, the encode line will cause an error.
body = 'sometext with UTF-8 André '.force_encoding('ASCII-8BIT')
body.encode('US-ASCII') # will raise Encoding::UndefinedConversionError:"\xC3" to UTF-8 in conversion from ASCII-8BIT to UTF-8 to US-ASCII
body.encode('UTF-8') # will raise Encoding::UndefinedConversionError: "\xC3" from ASCII-8BIT to UTF-8
body.encode('ASCII-8BIT') # raises nothingThere are a few things that I see need to be changed here.
- the error class should not be instantiated if there is no error detected (thus avoiding 99% of this nonsense)
- The re-encoding should simply not happen, OR it should have a "replace" mode activated e.g.
response_body.encode(script_encoding, :invalid => :replace, :undef => :replace)
Right now we have customers who can not authenticate with Asana due to the name André appearing in the response data, and this unnecessary error class being created causing an error.