curtin-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/curtin:fix/udevadm-info-shlex-quote into curtin:master.

Commit message:
udev: use shlex.quote before shlex.split to preserve shell escape chars

The udev database includes shell escape characters in the output.
Use shlex.quote on the values before calling shlex.split to preserve
the original characture in the value.

- Python2.7 shlex does not implement quote, use a best approximation

LP: #1875085


Requested reviews:
  Dan Watkins (daniel-thewatkins)
  Shawn Weeks (absolutesantaja)
  Server Team CI bot (server-team-bot): continuous-integration
Related bugs:
  Bug #1875085 in curtin (Ubuntu): "curtin install hook fails with No closing quotation on new 20.04 install"
  https://bugs.launchpad.net/ubuntu/+source/curtin/+bug/1875085

For more details, see:
https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/382993
-- 
Your team curtin developers is subscribed to branch curtin:master.

diff --git a/curtin/udev.py b/curtin/udev.py
index e2e3dd0..0908bc9 100644
--- a/curtin/udev.py
+++ b/curtin/udev.py
@@ -4,7 +4,14 @@ import shlex
 import os
 
 from curtin import util
-from curtin.log import logged_call
+from curtin.log import logged_call, LOG
+
+try:
+    shlex_quote = shlex.quote
+except AttributeError:
+    # python2.7 shlex does not have quote, give it a try
+    def shlex_quote(value):
+        return ("'" + value.replace("'", "\'\"\'\"\'") + "'")
 
 
 def compose_udev_equality(key, value):
@@ -90,7 +97,22 @@ def udevadm_info(path=None):
             value = None
         if value:
             # preserve spaces in values to match udev database
-            parsed = shlex.split(value)
+            try:
+                parsed = shlex.split(value)
+            except ValueError:
+                try:
+                    # strip the leading/ending single tick from udev output
+                    quoted = shlex_quote(value[1:-1])
+                    LOG.debug('udevadm_info: quoting shell-escape chars '
+                              'in %s=%s -> %s', key, value, quoted)
+                    parsed = shlex.split(quoted)
+                except ValueError:
+                    # strip the leading/ending single tick from udev output
+                    escaped_value = (
+                        value[1:-1].replace("'", "_").replace('"', "_"))
+                    LOG.debug('udevadm_info: replacing shell-escape chars '
+                              'in %s=%s -> %s', key, value, escaped_value)
+                    parsed = shlex.split(escaped_value)
             if ' ' not in value:
                 info[key] = parsed[0]
             else:
diff --git a/tests/data/udevadm_info_sandisk_cruzer.txt b/tests/data/udevadm_info_sandisk_cruzer.txt
new file mode 100644
index 0000000..a605afe
--- /dev/null
+++ b/tests/data/udevadm_info_sandisk_cruzer.txt
@@ -0,0 +1,54 @@
+DEVPATH='/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.5/2-1.5.1/2-1.5.1:1.0/host6/target6:0:0/6:0:0:0/block/sdc/sdc1'
+DEVNAME='/dev/sdc1'
+DEVTYPE='partition'
+PARTN='1'
+MAJOR='8'
+MINOR='33'
+SUBSYSTEM='block'
+USEC_INITIALIZED='5265867'
+SCSI_TPGS='0'
+SCSI_TYPE='disk'
+SCSI_VENDOR='SanDisk''
+SCSI_VENDOR_ENC='SanDisk''
+SCSI_MODEL='Cruzer_Fit'
+SCSI_MODEL_ENC='Cruzer\x20Fit\x20\x20\x20\x20\x20\x20'
+SCSI_REVISION='1.00'
+ID_SCSI='1'
+ID_VENDOR='SanDisk_'
+ID_VENDOR_ENC='SanDisk\x27'
+ID_MODEL='Cruzer_Fit'
+ID_MODEL_ENC='Cruzer\x20Fit\x20\x20\x20\x20\x20\x20'
+ID_REVISION='1.00'
+ID_TYPE='disk'
+ID_SCSI_INQUIRY='1'
+ID_VENDOR_ID='0781'
+ID_MODEL_ID='5571'
+ID_SERIAL='SanDisk__Cruzer_Fit_4C530000140118216265-0:0'
+ID_SERIAL_SHORT='4C530000140118216265'
+ID_INSTANCE='0:0'
+ID_BUS='usb'
+ID_USB_INTERFACES=':080650:'
+ID_USB_INTERFACE_NUM='00'
+ID_USB_DRIVER='usb-storage'
+ID_PATH='pci-0000:00:1d.0-usb-0:1.5.1:1.0-scsi-0:0:0:0'
+ID_PATH_TAG='pci-0000_00_1d_0-usb-0_1_5_1_1_0-scsi-0_0_0_0'
+ID_PART_TABLE_UUID='36b64baf'
+ID_PART_TABLE_TYPE='dos'
+ID_FS_UUID='2020-04-23-08-02-07-00'
+ID_FS_UUID_ENC='2020-04-23-08-02-07-00'
+ID_FS_BOOT_SYSTEM_ID='EL\x20TORITO\x20SPECIFICATION'
+ID_FS_VERSION='Joliet Extension'
+ID_FS_LABEL='Ubuntu-Server_20.04_LTS_amd64'
+ID_FS_LABEL_ENC='Ubuntu-Server\x2020.04\x20LTS\x20amd64'
+ID_FS_TYPE='iso9660'
+ID_FS_USAGE='filesystem'
+ID_PART_ENTRY_SCHEME='dos'
+ID_PART_ENTRY_UUID='36b64baf-01'
+ID_PART_ENTRY_TYPE='0x0'
+ID_PART_ENTRY_FLAGS='0x80'
+ID_PART_ENTRY_NUMBER='1'
+ID_PART_ENTRY_OFFSET='0'
+ID_PART_ENTRY_SIZE='1859584'
+ID_PART_ENTRY_DISK='8:32'
+DEVLINKS='/dev/disk/by-path/pci-0000:00:1d.0-usb-0:1.5.1:1.0-scsi-0:0:0:0-part1 /dev/disk/by-id/usb-SanDisk__Cruzer_Fit_4C530000140118216265-0:0-part1 /dev/disk/by-label/Ubuntu-Server\x2020.04\x20LTS\x20amd64 /dev/disk/by-partuuid/36b64baf-01 /dev/disk/by-uuid/2020-04-23-08-02-07-00'
+TAGS=':systemd:'
diff --git a/tests/unittests/test_udev.py b/tests/unittests/test_udev.py
index 33d5f44..0c6c5b9 100644
--- a/tests/unittests/test_udev.py
+++ b/tests/unittests/test_udev.py
@@ -8,7 +8,7 @@ from .helpers import CiTestCase
 
 
 UDEVADM_INFO_QUERY = """\
-DEVLINKS='/dev/disk/by-id/nvme-eui.0025388b710116a1'
+DEVLINKS='/dev/disk/by-id/nvme-eui.0025388b710116a1 /dev/disk/by-id/nvme-n1'
 DEVNAME='/dev/nvme0n1'
 DEVPATH='/devices/pci0000:00/0000:00:1c.4/0000:05:00.0/nvme/nvme0/nvme0n1'
 DEVTYPE='disk'
@@ -24,7 +24,8 @@ USEC_INITIALIZED='2026691'
 """
 
 INFO_DICT = {
-    'DEVLINKS': ['/dev/disk/by-id/nvme-eui.0025388b710116a1'],
+    'DEVLINKS': ['/dev/disk/by-id/nvme-eui.0025388b710116a1',
+                 '/dev/disk/by-id/nvme-n1'],
     'DEVNAME': '/dev/nvme0n1',
     'DEVPATH':
         '/devices/pci0000:00/0000:00:1c.4/0000:05:00.0/nvme/nvme0/nvme0n1',
@@ -54,6 +55,19 @@ class TestUdevInfo(CiTestCase):
             capture=True)
         self.assertEqual(sorted(INFO_DICT), sorted(info))
 
+    @mock.patch('curtin.util.subp')
+    def test_udevadm_info_escape_quotes(self, m_subp):
+        """verify we escape quotes when we fail to split. """
+        mypath = '/dev/sdz'
+        datafile = 'tests/data/udevadm_info_sandisk_cruzer.txt'
+        m_subp.return_value = (util.load_file(datafile), "")
+        info = udev.udevadm_info(mypath)
+        m_subp.assert_called_with(
+            ['udevadm', 'info', '--query=property', '--export', mypath],
+            capture=True)
+        self.assertEqual('SanDisk'"'"'', info['SCSI_VENDOR'])
+        self.assertEqual('SanDisk'"'"'', info['SCSI_VENDOR_ENC'])
+
     def test_udevadm_info_no_path(self):
         """ udevadm_info raises ValueError for invalid path value"""
         mypath = None