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

Fix UTF-8, ASCII-8BIT Compatibility Error [#139953217] #74

Merged
merged 2 commits into from Feb 17, 2017

Conversation

@conradbeach
Copy link
Contributor

commented Feb 16, 2017

Zip::Entry#name returns an ASCII-8BIT encoded string. Instances of SenkyoshiFile had this encoding for the strings in @xid and @path. That had the potential to cause compatibility issues. This forces UTF-8 encoding on those instance variables.

It's worth noting that Zip::Entry#name is called elsewhere by Senkyoshi. It might be necessary to force UTF-8 encoding in other places as well. I chose just to fix the issue we were actually experiencing for now.

Conrad Beach
Fix UTF-8, ASCII-8BIT compatability error.
Zip::Entry.name returns an ASCII-8BIT encoded string. Instances of SenkyoshiFile had this encoding for the strings in @xid and @path. This had the potential to cause compatability issues. This forces UTF-8 encoding on those instance variables.

@conradbeach conradbeach changed the title Fix UTF-8, ASCII-8BIT Compatibility Error Fix UTF-8, ASCII-8BIT Compatibility Error [#139953217] Feb 16, 2017

@conradbeach

This comment has been minimized.

Copy link
Contributor Author

commented Feb 16, 2017

I looked into using encode instead of force_encoding, but it looks like encode is too gentle. On the example course, it errors out about a quarter of the time saying it can't perform the conversion.

What I decided to do is try encode first and then use force_encoding after that if encode doesn't work. Thoughts?

@srphillips

This comment has been minimized.

Copy link
Contributor

commented Feb 17, 2017

Do we know which characters it's failing on specifically?

@conradbeach

This comment has been minimized.

Copy link
Contributor Author

commented Feb 17, 2017

\xE2

The full error message (for every error) is this:<Encoding::UndefinedConversionError: "\xE2" from ASCII-8BIT to UTF-8>

@conradbeach

This comment has been minimized.

Copy link
Contributor Author

commented Feb 17, 2017

We could specifically handle that character and covert it manually. encode gives you a way to do that. I think we'd still want to fall back on force_encoding though. This is just one course and there might be instances of other problem characters at some point.

I'm not sure trying to manually convert the character will be all that valuable though, force_encoding is already making a reasonable conversion.

@srphillips

This comment has been minimized.

Copy link
Contributor

commented Feb 17, 2017

Agreed that it might come up in other places, just curious about why it's still failing. force_encoding is fine for now.

@srphillips
Copy link
Contributor

left a comment

Approved!

@conradbeach

This comment has been minimized.

Copy link
Contributor Author

commented Feb 17, 2017

Sounds good. Thanks.

@conradbeach conradbeach merged commit b6cf659 into master Feb 17, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!

@conradbeach conradbeach deleted the ckb-fix-encoding branch Feb 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.