mountinfo: fix path unescaping

[kolyshkin]

Moby PR moby/moby#38977 and containerd PR containerd/containerd#3159
added the handing of escape sequences in mountinfo paths (root and mountpoint fields).

Unfortunately, it also broke the handling of paths containing double
quotes, as it was pointed out in containerd/containerd#4257

The solution is to stop using strconv.Unquote and write our own
specialized function to deal with escape sequences.

Unit tests added.

In addition, unescape the fstype and source fields (the first two after the - separator, since kernel escapes those as well.

[kolyshkin]

@thaJeztah PTAL

[thaJeztah]

LGTM, thanks!

[thaJeztah]

@cpuguy83 PTAL

[cpuguy83]

LGTM

Nit about function doc.
Might be nice to publish this for other uses as well (just a thought).

[kolyshkin]

Might be nice to publish this for other uses as well (just a thought).

Maybe, but I don't want it to be part of mountinfo. We can make something like moby/sys/filepath or moby/sys/linux or something like that later.

[kolyshkin]

Added a second commit, unescaping the fstype and source fields.

[cpuguy83]

LGTM

[thaJeztah]

LGTM

[zhsj]

I'm not sure what happens. The CI seems happy. But I fail to run test locally now.

$ git bisect run bash -c 'cd mountinfo; go test -run TestParseMountinfoWithSpaces'
running bash -c cd mountinfo; go test -run TestParseMountinfoWithSpaces
PASS
ok      github.com/moby/sys/mountinfo   0.002s
Bisecting: 1 revision left to test after this (roughly 1 step)
[2dee25b83acf7cab475f26a3a1da8b2a9a03e833] Merge pull request #16 from kolyshkin/fix-unescape
running bash -c cd mountinfo; go test -run TestParseMountinfoWithSpaces
--- FAIL: TestParseMountinfoWithSpaces (0.00s)
    mountinfo_linux_test.go:537: expected mountinfo.Info{ID:31, Parent:21, Major:0, Minor:23, Root:"/", Mountpoint:"/DATA/foo_bla_bla", Opts:"rw,relatime", Optional:"", Fstype:"cifs", Source:"//foo/BLA\\040BLA\\040BLA/", VfsOpts:"rw,sec=ntlm,cache=loose,unc=\\\\foo\\BLA"}, got &mountinfo.Info{ID:31, Parent:21, Major:0, Minor:23, Root:"/", Mountpoint:"/DATA/foo_bla_bla", Opts:"rw,relatime", Optional:"", Fstype:"cifs", Source:"//foo/BLA BLA BLA/", VfsOpts:"rw,sec=ntlm,cache=loose,unc=\\\\foo\\BLA"}
FAIL
exit status 1
FAIL    github.com/moby/sys/mountinfo   0.002s
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[0b171c288ece8a72ad48ee827ecaa4e53091e8d5] mountinfo: also unescape fstype and source
running bash -c cd mountinfo; go test -run TestParseMountinfoWithSpaces
--- FAIL: TestParseMountinfoWithSpaces (0.00s)
    mountinfo_linux_test.go:537: expected mountinfo.Info{ID:31, Parent:21, Major:0, Minor:23, Root:"/", Mountpoint:"/DATA/foo_bla_bla", Opts:"rw,relatime", Optional:"", Fstype:"cifs", Source:"//foo/BLA\\040BLA\\040BLA/", VfsOpts:"rw,sec=ntlm,cache=loose,unc=\\\\foo\\BLA"}, got &mountinfo.Info{ID:31, Parent:21, Major:0, Minor:23, Root:"/", Mountpoint:"/DATA/foo_bla_bla", Opts:"rw,relatime", Optional:"", Fstype:"cifs", Source:"//foo/BLA BLA BLA/", VfsOpts:"rw,sec=ntlm,cache=loose,unc=\\\\foo\\BLA"}
FAIL
exit status 1
FAIL    github.com/moby/sys/mountinfo   0.002s
0b171c288ece8a72ad48ee827ecaa4e53091e8d5 is the first bad commit
commit 0b171c288ece8a72ad48ee827ecaa4e53091e8d5
Author: Kir Kolyshkin <kolyshkin@gmail.com>
Date:   Tue Jun 23 14:12:13 2020 -0700

    mountinfo: also unescape fstype and source
    
    Just noticed it while reading the kernel source code (function
    show_mountinfo() in fs/proc_namespace.c; see also: show_type(),
    mangle()) that the first two fields after the `-` separator are also
    escaped.
    
    While I haven't seen it in real life, it's better we unescape them.
    
    No field-specific tests were added, and unescape() is tested by its own
    test case.
    
    Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

 mountinfo/mountinfo_linux.go | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
bisect run success

[zhsj]

The CI is false positive...

[thaJeztah]

Looks like this is the test that's failing

[thaJeztah]

Wondering why CI didn't fail; could it be because we're using a sub shell?

sys/Makefile

Line 10 in 4a24c04

(cd $$p && go test -v .); \

Hm.. no idea; trying to reproduce with a very basic Makefile;

PACKAGES ?= mountinfo mount

.PHONY: test
test:
        for p in $(PACKAGES); do \
                (echo $$p && exit 1); \
        done

make test
for p in mountinfo; do \
		(echo $p && exit 1); \
	done
mountinfo
make: *** [test] Error 1

[zhsj]

@thaJeztah I have proposed the patch for CI at https://github.com/moby/sys/pull/19/files#diff-b67911656ef5d18c4ae36cb6741b7965R1

[thaJeztah]

@zhsj thanks! just saw it after I posted my last comment