Merge ~chad.smith/cloud-init:fix/1768600-utf8-in-user-data into cloud-init:master

Proposed by Chad Smith
Status: Merged
Approved by: Scott Moser
Approved revision: d6262a2e83ecc5277eab30fad374aaa7602b41c1
Merge reported by: Scott Moser
Merged at revision: faa6f07e9de4058083a5f69ed508b6e24bd53b23
Proposed branch: ~chad.smith/cloud-init:fix/1768600-utf8-in-user-data
Merge into: cloud-init:master
Diff against target: 72 lines (+23/-10)
2 files modified
cloudinit/user_data.py (+13/-9)
tests/unittests/test_data.py (+10/-1)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Ryan Harper Approve
Review via email: mp+347782@code.launchpad.net

Commit message

Be more safe on string/bytes when writing multipart user-data to disk.

When creating the multipart mime message that is written as
user-data.txt.i, cloud-init losing data on conversion to some things
as a string.

LP: #1768600

Author: Scott Moser <email address hidden>
Co-Authored-By: Chad Smith <email address hidden>

Description of the change

Took over Scott's branch https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/347727 and added/tweaked a couple of unit tests to show failure path on master without this branch.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:d6a4ebb6cc4ab55debe0caea4ebaf28178717bac
https://jenkins.ubuntu.com/server/job/cloud-init-ci/67/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/67/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

Whitespace question/nit. I'm fine either way.

review: Approve
867db9c... by Chad Smith

Drop unicode character from docstr in unit test.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:e6233f16e6ea8b78b7ed1446cee88b1c9277608f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/68/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/68/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:867db9cb78bddd3bed70146fd0d2c67c49bb1628
https://jenkins.ubuntu.com/server/job/cloud-init-ci/69/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/69/rebuild

review: Needs Fixing (continuous-integration)
d6262a2... by Chad Smith

pycodestyle spacing fix

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:d6262a2e83ecc5277eab30fad374aaa7602b41c1
https://jenkins.ubuntu.com/server/job/cloud-init-ci/79/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/79/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) :
review: Approve
Revision history for this message
Scott Moser (smoser) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=faa6f07e

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/user_data.py b/cloudinit/user_data.py
2index 8f6aba1..ed83d2d 100644
3--- a/cloudinit/user_data.py
4+++ b/cloudinit/user_data.py
5@@ -337,8 +337,10 @@ def is_skippable(part):
6
7 # Coverts a raw string into a mime message
8 def convert_string(raw_data, content_type=NOT_MULTIPART_TYPE):
9+ """convert a string (more likely bytes) or a message into
10+ a mime message."""
11 if not raw_data:
12- raw_data = ''
13+ raw_data = b''
14
15 def create_binmsg(data, content_type):
16 maintype, subtype = content_type.split("/", 1)
17@@ -346,15 +348,17 @@ def convert_string(raw_data, content_type=NOT_MULTIPART_TYPE):
18 msg.set_payload(data)
19 return msg
20
21- try:
22- data = util.decode_binary(util.decomp_gzip(raw_data))
23- if "mime-version:" in data[0:4096].lower():
24- msg = util.message_from_string(data)
25- else:
26- msg = create_binmsg(data, content_type)
27- except UnicodeDecodeError:
28- msg = create_binmsg(raw_data, content_type)
29+ if isinstance(raw_data, six.text_type):
30+ bdata = raw_data.encode('utf-8')
31+ else:
32+ bdata = raw_data
33+ bdata = util.decomp_gzip(bdata, decode=False)
34+ if b"mime-version:" in bdata[0:4096].lower():
35+ msg = util.message_from_string(bdata.decode('utf-8'))
36+ else:
37+ msg = create_binmsg(bdata, content_type)
38
39 return msg
40
41+
42 # vi: ts=4 expandtab
43diff --git a/tests/unittests/test_data.py b/tests/unittests/test_data.py
44index 91d35cb..3efe7ad 100644
45--- a/tests/unittests/test_data.py
46+++ b/tests/unittests/test_data.py
47@@ -606,8 +606,10 @@ class TestUDProcess(helpers.ResourceUsingTestCase):
48
49
50 class TestConvertString(helpers.TestCase):
51+
52 def test_handles_binary_non_utf8_decodable(self):
53- blob = b'\x32\x99'
54+ """Printable unicode (not utf8-decodable) is safely converted."""
55+ blob = b'#!/bin/bash\necho \xc3\x84\n'
56 msg = ud.convert_string(blob)
57 self.assertEqual(blob, msg.get_payload(decode=True))
58
59@@ -621,6 +623,13 @@ class TestConvertString(helpers.TestCase):
60 msg = ud.convert_string(text)
61 self.assertEqual(text, msg.get_payload(decode=False))
62
63+ def test_handle_mime_parts(self):
64+ """Mime parts are properly returned as a mime message."""
65+ message = MIMEBase("text", "plain")
66+ message.set_payload("Just text")
67+ msg = ud.convert_string(str(message))
68+ self.assertEqual("Just text", msg.get_payload(decode=False))
69+
70
71 class TestFetchBaseConfig(helpers.TestCase):
72 def test_only_builtin_gets_builtin(self):

Subscribers

People subscribed via source and target branches