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 task results #17252

Merged
merged 1 commit into from
Apr 6, 2018

Conversation

imtayadeway
Copy link
Contributor

Since task results are stored as binary blobs, they can be set as
strings in whatever encoding you like. This can create problems for
the API, which blindly tries to convert all attributes to JSON - any
value containing ASCII with ansi escape codes will fail with a
conversion error.

It seems reasonable to expect that all string values should be encoded
as UTF-8 - the alternative is for the API to have to force the
encoding on every single string that comes its way.

This change forces incoming values to be converted to UTF-8 prior to
being stored. It will also convert outgoing values in the case that
they have already been persisted prior to this change - though this
extra check I'm sure could eventually be removed.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1557731

Since task results are stored as binary blobs, they can be set as
strings in whatever encoding you like. This can create problems for
the API, which blindly tries to convert all attributes to JSON - any
value containing ASCII with ansi escape codes will fail with a
conversion error.

It seems reasonable to expect that all string values should be encoded
as UTF-8 - the alternative is for the API to have to force the
encoding on every single string that comes its way.

This change forces incoming values to be converted to UTF-8 prior to
being stored. It will also convert outgoing values in the case that
they have already been persisted prior to this change - though this
extra check I'm sure could eventually be removed.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1557731
@miq-bot
Copy link
Member

miq-bot commented Apr 4, 2018

Checked commit imtayadeway@0559e91 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@gtanzillo gtanzillo added this to the Sprint 83 Ending Apr 9, 2018 milestone Apr 6, 2018
@gtanzillo gtanzillo merged commit c7d1580 into ManageIQ:master Apr 6, 2018
@simaishi
Copy link
Contributor

@imtayadeway gaprindashvili/yes ?

simaishi pushed a commit that referenced this pull request Apr 12, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit abe1095c02d8032ab9cbac777ba1d32c8815838d
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Fri Apr 6 08:18:44 2018 -0400

    Merge pull request #17252 from imtayadeway/force-task-results-encoding
    
    Force UTF-8 encoding on task results
    (cherry picked from commit c7d158064e5241e03a98a69cf53f01dae753a70b)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1566572

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants