Skip to content

Handle UnicodeDecodeError in check.#12

Merged
audreyfeldroy merged 1 commit intobinaryornot:masterfrom
DRMacIver:fix-key-error
Aug 16, 2015
Merged

Handle UnicodeDecodeError in check.#12
audreyfeldroy merged 1 commit intobinaryornot:masterfrom
DRMacIver:fix-key-error

Conversation

@DRMacIver
Copy link
Copy Markdown
Contributor

This file causes chardet to give a high confidence for an
encoding which python cannot decode the file as, which triggers
a UnicodeDecodeError. This should just result in is_binary returning
false but instead results in a KeyError.

The reason for this is that the error is thrown before the local
was assigned, and thus when it comes to formatting the string for
logging the local is not present and so you get the KeyError.

Note: This example was actually found by Hypothesis. So far it's not found any others that are definitely bugs (though there are a few suspicious examples it found I can give you), but I can also submit a pull request that adds some Hypothesis based tests if you want it.

This file causes chardet to give a high confidence for an
encoding which python cannot decode the file as, which triggers
a UnicodeDecodeError. This should just result in is_binary returning
false but instead results in a KeyError.

The reason for this is that the error is thrown before the local
was assigned, and thus when it comes to formatting the string for
logging the local is not present and so you get the KeyError.
audreyfeldroy added a commit that referenced this pull request Aug 16, 2015
Handle UnicodeDecodeError in check.
@audreyfeldroy audreyfeldroy merged commit a611311 into binaryornot:master Aug 16, 2015
@audreyfeldroy
Copy link
Copy Markdown
Collaborator

Thanks for catching this @DRMacIver. Hypothesis sounds amazing and you've piqued my curiosity with this...I'd love a PR with Hypothesis-based tests if you have time. I'm taking a look at the Hypothesis docs now.

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.

2 participants