bugfix/set character set on output stream writer to UTF 8 #15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
srt4
commented
Jan 30, 2018
| connection.setDoOutput(true); | ||
| connection.setRequestMethod(method); | ||
|
|
||
| writer = new OutputStreamWriter(connection.getOutputStream(), "UTF-8"); |
There was a problem hiding this comment.
This is the significant change. It calls the constructor with two parameters (previously it was just writer = new OutputStreamWriter(connection.getOutputStream());) - the second being the character set to support, overriding the system default (which shouldn't be used in an SDK).
srt4
commented
Jan 30, 2018
| /** | ||
| * This class represents an HTTP Request message. | ||
| */ | ||
| public class Utf8Request extends Message<Utf8Request> { |
There was a problem hiding this comment.
The class name is updated so that we can still import the cc.protea library while avoiding contentious class-loading cardinality issues.
gmeyer-hw
force-pushed
the
master
branch
3 times, most recently
from
155ac7b
to
e646a8f
Compare
|
Hi @srt4, Thanks for you contributing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The existing HTTP client from cc.protea uses an
OutputStreamWriterwith the one-argument constructer, leading it to accept the system default character set.This presents issues as Hyperwallet accepts a certain character set, but does not know which characters the client's host OS supports by default.
The change sets the character set to UTF-8, which is in accordance with Hyperwallet specifications.
n.b. I highly recommend that Hyperwallet migrates away from using the
cc.proteaHTTP library. The library is useful in that it's syntactic sugar forHttpURLConnection, but past that it's detrimental in terms of debuggability. I opened another pull request that also overrides a cc.protea class - and most of the functionality provided by thecc.protealibrary would be replicated by allowing theHyperwalletApiClientclass to accept an HttpClient parameter.