Skip to content

Fix error encoding #271

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 30, 2016
Merged

Fix error encoding #271

merged 4 commits into from
Nov 30, 2016

Conversation

@urkle
Copy link
Contributor

@urkle urkle commented Sep 21, 2016 •
edited
Loading

Fix for #270

This makes two changes

  • adds handling of unknown encoding bytes when re-encoding the error response
  • only instantiates the Error class if an actual error has occurred.

@urkle urkle force-pushed the fix-error-encoding branch 3 times, most recently from b7c0084 to 05e0df3 Compare September 28, 2016 18:09
@urkle
do not create an Error instance if there is no error
c89e19d 
- also re-validate in a test that encoding errors do not cause issues
@urkle urkle force-pushed the fix-error-encoding branch from 05e0df3 to 1280951 Compare September 28, 2016 18:19
@urkle
fix builds on ruby 1.8.7 and 1.9.3
b669826 
- drop rubocop from ruby 1.9.x builds as bundler doesn't allow the expresiveness needed for specifying different versions based on the platform. (see rubygems/bundler#751)
@urkle
ruby 1.8.7 no longer works with the force encoding call
3f1f129
@urkle urkle force-pushed the fix-error-encoding branch from 1280951 to 3f1f129 Compare September 28, 2016 18:21
@urkle
Copy link
Contributor Author

urkle commented Sep 28, 2016

@sferik I fixed the rubycop issues in this..

I also tweaks the Gmefile and .travis.yml as ruby 1.8.7 no longer tests correctly, and 1.9.3 requires restricting gem versions to pass.

If you could please review this and provide feedback, that would be awesome, as this has caused significant issues in our production environments.

@robertIngrum
Copy link

robertIngrum commented Oct 3, 2016

👍 Would be great to get this out, causing significant issues for us as well.

@urkle urkle mentioned this pull request Oct 3, 2016
Closed
@robertIngrum
Copy link

robertIngrum commented Oct 14, 2016

@sferik: Just pinging again, would love to see this on master.

@jeffcarbs
Copy link

jeffcarbs commented Nov 16, 2016

Is there anything holding this up? We're using this gem with Office365 which returns non-ascii in some cases and our integration is currently failing for some of our customers. Would appreciate this making it into a release 👍

@jwg2s
Copy link

jwg2s commented Nov 27, 2016

Anything we can do to help get this merged?

@robertIngrum
Copy link

robertIngrum commented Nov 29, 2016

I think @sferik is the only admin, so just ping and hope it gets noticed I guess.

@sferik sferik merged commit c9bbb9c into ruby-oauth:master Nov 30, 2016
@sferik
Copy link
Contributor

sferik commented Nov 30, 2016

Thanks for this fix!

@jwg2s
Copy link

jwg2s commented Nov 30, 2016

@sferik worth issuing a v1.2.1 release for this? Pretty bad bug

@sferik sferik mentioned this pull request Dec 24, 2016
Closed
@Edouard-chin Edouard-chin mentioned this pull request Jun 16, 2017
Merged
@sarojkh sarojkh mentioned this pull request Aug 16, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@urkle @robertIngrum @jeffcarbs @jwg2s @sferik