Skip to content

udev-rules: all values can contain escaped double quotes now#6890

Merged
keszybz merged 1 commit intosystemd:masterfrom
fbuihuu:udev-rule-escaped-double-quotes
Sep 28, 2017
Merged

udev-rules: all values can contain escaped double quotes now#6890
keszybz merged 1 commit intosystemd:masterfrom
fbuihuu:udev-rule-escaped-double-quotes

Conversation

@fbuihuu
Copy link
Copy Markdown
Contributor

@fbuihuu fbuihuu commented Sep 22, 2017

This is primarly useful to support escaped double quotes in PROGRAM or
IMPORT{program} directives.

The only possibilty before this patch was to use an external shell script but
this seems too cumbersome for trivial logics.

Fixes: #6835

@keszybz
Copy link
Copy Markdown
Member

keszybz commented Sep 24, 2017

Can you please add some tests for this? We have a pretty good set of test for udev rules, and a tricky change like this is much safer with proper CI.

@fbuihuu
Copy link
Copy Markdown
Contributor Author

fbuihuu commented Sep 25, 2017

Sure, I'll do that.

@fbuihuu fbuihuu force-pushed the udev-rule-escaped-double-quotes branch from 73094fc to 89f15a5 Compare September 26, 2017 07:43
@fbuihuu
Copy link
Copy Markdown
Contributor Author

fbuihuu commented Sep 26, 2017

@keszybz : done

Copy link
Copy Markdown
Member

@keszybz keszybz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please incorporate the following test:

diff --cc test/udev-test.pl
index f1326a3d44,f1326a3d44..e8cc85a6df
--- test/udev-test.pl
+++ test/udev-test.pl
@@@ -349,6 -349,6 +349,14 @@@ SUBSYSTEMS=="scsi", PROGRAM=="/bin/sh -
  EOF
          },
          {
++                desc            => "program arguments combined with escaped double quotes, part 3",
++                devpath         => "/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda/sda5",
++                exp_name        => "foo2" ,
++                rules           => <<EOF
++SUBSYSTEMS=="scsi", PROGRAM=="/bin/sh -c 'printf \\\"%s %s\\\" \\\"foo1 foo2\\\" \\\"foo3\\\"| grep \\\"foo1 foo2\\\"'", KERNEL=="sda5", SYMLINK+="%c{2}"
++EOF
++        },
++        {
                  desc            => "characters before the %c{N} substitution",
                  devpath         => "/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda/sda5",
                  exp_name        => "my-foo9" ,

This fails here :(

while (pos != NULL && pos[0] != '\0') {
if (pos[0] == '\'') {
/* do not separate quotes */
if (pos[0] == '\'' || pos[0] == '"') {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IN_SET(pos[0], '\'', '"')?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding your failing test case: good catch ! actually in the test cases "%s" needs to be escaped as well otherwise udev will do its string substitutions.

I'll fix that.

Thanks !

This is primarly useful to support escaped double quotes in PROGRAM or
IMPORT{program} directives.

The only possibilty before this patch was to use an external shell script but
this seems too cumbersome for trivial logics such as

 PROGRAM=="/bin/sh -c 'FOO=\"%s{model}\"; echo ${FOO:0:4}'"

or any similar shell constructs that needs to deals with patterns including
whitespaces.

As it's the case for single quote and for directives running a program, words
within escaped double quotes will be considered as a single argument.

Fixes: systemd#6835
@fbuihuu fbuihuu force-pushed the udev-rule-escaped-double-quotes branch from 89f15a5 to 8a247d5 Compare September 26, 2017 13:26
@fbuihuu
Copy link
Copy Markdown
Contributor Author

fbuihuu commented Sep 26, 2017

@keszybz new version force pushed. Thanks for your review !

@keszybz keszybz merged commit 7e760b7 into systemd:master Sep 28, 2017
@fbuihuu fbuihuu deleted the udev-rule-escaped-double-quotes branch September 28, 2017 08:59
SergioAtSUSE pushed a commit to SergioAtSUSE/systemd_systemd that referenced this pull request Jun 7, 2018
…#6890)

This is primarly useful to support escaped double quotes in PROGRAM or
IMPORT{program} directives.

The only possibilty before this patch was to use an external shell script but
this seems too cumbersome for trivial logics such as

 PROGRAM=="/bin/sh -c 'FOO=\"%s{model}\"; echo ${FOO:0:4}'"

or any similar shell constructs that needs to deals with patterns including
whitespaces.

As it's the case for single quote and for directives running a program, words
within escaped double quotes will be considered as a single argument.

Fixes: systemd#6835
(cherry picked from commit 7e760b7)
Werkov pushed a commit to Werkov/systemd that referenced this pull request Nov 27, 2018
…#6890)

This is primarly useful to support escaped double quotes in PROGRAM or
IMPORT{program} directives.

The only possibilty before this patch was to use an external shell script but
this seems too cumbersome for trivial logics such as

 PROGRAM=="/bin/sh -c 'FOO=\"%s{model}\"; echo ${FOO:0:4}'"

or any similar shell constructs that needs to deals with patterns including
whitespaces.

As it's the case for single quote and for directives running a program, words
within escaped double quotes will be considered as a single argument.

Fixes: systemd#6835
(cherry picked from commit 7e760b7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants