[PATCH] Failed test: scm_rights-fd.test for path containing '>', '}' or non-printable characters

[Jay Joshi]

On Wed, Mar 23, 2016 at 11:01 PM, Dmitry V. Levin <ldv at altlinux.org> wrote:
> On Wed, Mar 23, 2016 at 10:29:37PM +0530, Jay Joshi wrote:
>> On Wed, Mar 23, 2016 at 8:53 PM, Dmitry V. Levin <ldv at altlinux.org> wrote:
>> > On Wed, Mar 23, 2016 at 07:44:24PM +0530, Jay Joshi wrote:
>> >> On Wed, Mar 23, 2016 at 1:24 AM, Dmitry V. Levin <ldv at altlinux.org> wrote:
>> >> > On Tue, Mar 22, 2016 at 09:52:55PM +0530, Jay Joshi wrote:
>> >> >> On Tue, Mar 22, 2016 at 5:29 AM, Dmitry V. Levin <ldv at altlinux.org> wrote:
>> >> >> > On Mon, Mar 21, 2016 at 05:18:10PM +0530, Jay Joshi wrote:
>> >> >> >> >From d718d0c8e94cab8a232caf13e282fdea29b9a8eb Mon Sep 17 00:00:00 2001
>> >> >> >> From: JayRJoshi <jay.r.joshi100 at gmail.com>
>> >> >> >> Date: Mon, 21 Mar 2016 17:15:11 +0530
>> >> >> >> Subject: [PATCH] fix pathname containing >,{ and other non-printables
>> >> >> >
>> >> >> > Good catch.
>> >> >> >
>> >> >> >> tests/scm_rights-fd.test: changed regexp for pathname in $rights
>> >> >> >> ---
>> >> >> >>  tests/scm_rights-fd.test | 2 +-
>> >> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >> >>
>> >> >> >> diff --git a/tests/scm_rights-fd.test b/tests/scm_rights-fd.test
>> >> >> >> index 48c5028..469348b 100755
>> >> >> >> --- a/tests/scm_rights-fd.test
>> >> >> >> +++ b/tests/scm_rights-fd.test
>> >> >> >> @@ -54,7 +54,7 @@ run_strace -y -x -enetwork $args "$file"
>> >> >> >>  sample='\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff'
>> >> >> >>  n='[1-9][0-9]*'
>> >> >> >>  msg='\{msg_name\(0\)=NULL, msg_iov\(1\)=\[\{"'"$sample"'", 15\}\],
>> >> >> >> msg_controllen='"$n"
>> >> >> >> -rights='\{cmsg_len='"$n"', cmsg_level=SOL_SOCKET,
>> >> >> >> cmsg_type=SCM_RIGHTS, \[4</dev/null>, 5</dev/zero>,
>> >> >> >> 6</[^}>]*/(A\\n){127}Z>\]\}'
>> >> >> >> +rights='(\{cmsg_len='"$n"', cmsg_level=SOL_SOCKET,
>> >> >> >> cmsg_type=SCM_RIGHTS, \[4</dev/null>, 5</dev/zero>,
>> >> >> >> 6</[^\0]*/(A\\n){127}Z>\]\}|\{cmsg_len='"$n"', cmsg_level=SOL_SOCKET,
>> >> >> >> cmsg_type=SCM_RIGHTS, \[4</dev/null>, 5</dev/zero>,
>> >> >> >> 6<(\\x[0-9a-f][0-9a-f])*\\x2f(\\x41\\x0a){127}\\x5a>\]\})'
>> >> >> >
>> >> >> > You don't have to clone the whole regexp if the only thing that has to be
>> >> >> > changed is the part of regexp related to 6<...>.
>> >> >> >
>> >> >> > BTW, I don't see how your change fixes the issue.
>> >> >>
>> >> >> First part of "or" was to match for path-name containing printable
>> >> >> characters only. For second part, string_quote while "x" flag set
>> >> >> print whole string in hexadecimal if at least one of the character is
>> >> >> is non-printable.
>> >> >
>> >> > So there are two bugs in the test, one is that '>' and '}' in $PWD are not
>> >> > handled properly, another one is that similar issue arises with those
>> >> > symbols in $PWD for which strace -x turns into hex mode.
>> >>
>> >> Yes. when path-name contains non-printable, at that time usehex is
>> >> set, which turns hex mode on.
>> >>
>> >> >> Updated patch is pasted. Am I missing something?
>> >> > [...]
>> >> >> 6<(/[^\0]*/(A\\n){127}Z|\\x2f(\\x[0-9a-f][0-9a-f])*\\x2f(\\x41\\x0a){127}\\x5a)>\]\}'
>> >> >
>> >> > What did you mean by [^\0]* ?  It doesn't work for me, while simple .* does.
>> >>
>> >> Yes, it doesn't make much sense. More appropriate expression would be
>> >> [^\0]+, so that at least path wouldn't start with "//". Although,
>> >> path-name containing "//" is valid. So, .* is better.
>> >
>> > OK, are you going to submit another fix for this issue?
>> >
>>
>> Okay.
>> I've few questions:
>> Why filename is chosen as sequence of A\n followed by Z?
>
> It came from getdents tests.  I needed a sequence with symbols that would
> have to be quoted but at the same time wouldn't turn usehex on.
>
>> Is it a good idea to replace:
>> sample='\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff'
>> with
>> sample="$(for i in $(seq 1 15); do printf '\\\\xf%x' $i; done)",
>
> This string came from tests/scm_rights.c.
> Yes, the shorter edition looks better.
>
>> and
>> socket=<socket:\\[[0-9]+\\]> and substitute it, to prevent text-overflow?
>
> Definitely.
>

>From 03529696dce8254555f01a4ecc9a3bc326926fbf Mon Sep 17 00:00:00 2001
From: JayRJoshi <jay.r.joshi100 at gmail.com>
Date: Thu, 24 Mar 2016 00:43:13 +0530
Subject: [PATCH] tests: fix pathname regex

Pathname containing >,} or non-printable characters was resulting in
failing of test.

* tests/scm_rights-fd.test: fix pathname regex and simplify things
---
 tests/scm_rights-fd.test | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/tests/scm_rights-fd.test b/tests/scm_rights-fd.test
index 48c5028..db1eeeb 100755
--- a/tests/scm_rights-fd.test
+++ b/tests/scm_rights-fd.test
@@ -51,16 +51,26 @@ touch -- "$file" ||
 run_prog ./scm_rights /dev/zero
 run_strace -y -x -enetwork $args "$file"

-sample='\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff'
+hex='[0-9a-f]'
 n='[1-9][0-9]*'
-msg='\{msg_name\(0\)=NULL, msg_iov\(1\)=\[\{"'"$sample"'", 15\}\],
msg_controllen='"$n"
-rights='\{cmsg_len='"$n"', cmsg_level=SOL_SOCKET,
cmsg_type=SCM_RIGHTS, \[4</dev/null>, 5</dev/zero>,
6</[^}>]*/(A\\n){127}Z>\]\}'
-creds='\{cmsg_len='"$n"', cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, \{pid='"$n"', uid=[0-9]+, gid=[0-9]+\}\}'
+sample="$(for i in $(seq 1 15); do printf '\\\\xf%x' $i; done)"
+socket='<socket:\[[0-9]+\]>'
+
+path6='(/.*/(A\\n){127}Z|\\x2f(\\x'"$hex""$hex"')*\\x2f(\\x41\\x0a){127}\\x5a)'
+cmsg_head='cmsg_len='"$n"', cmsg_level=SOL_SOCKET'
+rights_data='\[4</dev/null>, 5</dev/zero>, 6<'"$path6"'>\]'
+iov='\[\{"'"$sample"'", 15\}\]'
+ids='\{pid='"$n"', uid=[0-9]+, gid=[0-9]+\}'
+
+msg='\{msg_name\(0\)=NULL, msg_iov\(1\)='"$iov"', msg_controllen='"$n"
+rights='\{'"$cmsg_head"', cmsg_type=SCM_RIGHTS, '"$rights_data"'\}'
+creds='\{cmsg_len='"$n"', cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, '"$ids"'\}'
+
 EXPECTED="$LOG.expected"

 cat > "$EXPECTED" << __EOF__
-sendmsg\\(3<socket:\\[[0-9]+\\]>, $msg, \\[$rights\\],
msg_flags=0\\}, 0\\) = 15
-recvmsg\\(0<socket:\\[[0-9]+\\]>, $msg, \\[$creds, $rights\\],
msg_flags=0\\}, 0\\) = 15
+sendmsg\\(3$socket, $msg, \\[$rights\\], msg_flags=0\\}, 0\\) = 15
+recvmsg\\(0$socket, $msg, \\[$creds, $rights\\], msg_flags=0\\}, 0\\) = 15
 __EOF__

 match_grep "$LOG" "$EXPECTED"
-- 
1.9.1