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

Check if byte strings are properly encoded in UTF-8 #30424

Merged
merged 5 commits into from Jan 19, 2016

Conversation

isbm
Copy link
Contributor

@isbm isbm commented Jan 18, 2016

Sometimes 3rd party packages contains broken UTF-8 strings. This renders JSON output crash (although works on STDOUT and YAML). This PR fixes this problem and logs the error log about bad package.

@cachedout
Copy link
Contributor

Thanks for sending this in @isbm

Could you please clarify what you mean by "broken UTF-8"? If those strings are just simply not UTF-8 and the underlying package manager supports them, it seems like we should try to as well. Thoughts?

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jan 19, 2016
@isbm
Copy link
Contributor Author

isbm commented Jan 19, 2016

@cachedout Well, some 3rd-party packagers entering, say, German umlauts or something else (Kanji?) and those symbols aren't properly converted or weren't properly stored (wrong editor, locale etc). Often it happens in the "Description" and "Summary" fields of the package. We also found few packages in our codebase this way. So in order to hunt all them down, prevent them in a future and ask maintainer to fix it, this PR helps to do so. However, typical API user will just get a description as is (just without these symbols).

I hope it makes sense.

@cachedout
Copy link
Contributor

@isbm Sounds good. Thanks for the clarification.

cachedout pushed a commit that referenced this pull request Jan 19, 2016
Check if byte strings are properly encoded in UTF-8
@cachedout cachedout merged commit 05ad3dc into saltstack:2015.8 Jan 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants