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

Force UTF-8 encoding on incoming events #16

Merged
merged 1 commit into from
Jan 11, 2019

Conversation

MusikAnimal
Copy link
Contributor

If the "chunk" contains characters that aren't UTF-8, the library will fail with "invalid byte sequence in UTF-8 (ArgumentError)". This patch fixes this and ensures we work only with UTF-8.

@francois2metz
Copy link
Owner

Hi. Thanks for the contribution.

i'm fine with forcing UTF-8 (it's in the spec), but why encoding it as UTF-16be first?

@MusikAnimal
Copy link
Contributor Author

MusikAnimal commented Dec 27, 2018

Apparently with older Rubies (pre 2.1), encode is a no-op if the string has the same encoding, so you have to encode into a different encoding then back to utf-8. Ruby pre-2.1 is really old, so maybe we don't care? This is deserving of a comment, at least.

The more modern solution I think would use scrub (introduced with 2.1), as with chunk.scrub('?').

In my applications I use str.force_encoding('utf-8') but that didn't seem to work here, for whatever reason.

@francois2metz
Copy link
Owner

Ruby 2.1 is no longer maintained, so I'm ok to drop support of it.

Could you add a testcase please?

Then I'll check what browsers do in this case.

@MusikAnimal
Copy link
Contributor Author

Sure thing, though I'm not sure how to run the tests. Is this standard rspec? With rspec spec/ I get the errors undefined method `assert_equal' for nil:NilClass

I may not get back to this until the new year, FYI. Thanks for the help!

@MusikAnimal
Copy link
Contributor Author

Never mind, I got the tests to run, and they pass :) I'll add a test case when I return January 1.

If the "chunk" contains characters that aren't UTF-8, the library
will fail with "invalid byte sequence in UTF-8 (ArgumentError)".
This patch fixes this and ensures we work only with UTF-8.
@MusikAnimal
Copy link
Contributor Author

@francois2metz Not sure if you were waiting on a ping. I added a test case :) No rush to merge obviously, just letting you know.

@francois2metz
Copy link
Owner

Hi. Thanks for letting me know. I'll take a look later.

@francois2metz francois2metz merged commit 5151e1f into francois2metz:master Jan 11, 2019
@francois2metz
Copy link
Owner

Thanks for the patch! 💛 💜 💙 💚

@francois2metz
Copy link
Owner

I released the version 0.3.1 with the fix.

@MusikAnimal MusikAnimal deleted the patch-1 branch January 11, 2019 17:45
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 this pull request may close these issues.

None yet

2 participants