Skip to content

[release/1.2 backport] Fix parseInfoFile does not handle spaces in filenames#3161

Merged
estesp merged 2 commits intocontainerd:release/1.2from
thaJeztah:1.2_backport_fix_parseinfofile_parsing
Apr 2, 2019
Merged

[release/1.2 backport] Fix parseInfoFile does not handle spaces in filenames#3161
estesp merged 2 commits intocontainerd:release/1.2from
thaJeztah:1.2_backport_fix_parseinfofile_parsing

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

backport of #3159 for the 1.2 branch

equivalent to moby/moby#38977, for containerd

relates to moby/moby#38458
relates to kubernetes/kubernetes#35062

/proc/self/mountinfo uses \040 for spaces, however, parseInfoFile()
did not decode those spaces in paths, therefore attempting to use \040
as a literal part of the path.

This patch un-quotes the root and mount point fields to fix
situations where paths contain spaces.

Note that the mount source field is not modified, given that
this field is documented (man PROC(5)) as:

filesystem-specific information or "none"

Which I interpreted as "the format in this field is undefined".

Thanks to @remexre for reporting, and @itizir, @sergei-utinski for suggesting the patch.

`/proc/self/mountinfo` uses `\040` for spaces, however, `parseInfoFile()`
did not decode those spaces in paths, therefore attempting to use `\040`
as a literal part of the path.

This patch un-quotes the `root` and `mount point` fields to fix
situations where paths contain spaces.

Note that the `mount source` field is not modified, given that
this field is documented (man `PROC(5)`) as:

    filesystem-specific information or "none"

Which I interpreted as "the format in this field is undefined".

Reported-by: Daniil Yaroslavtsev <[email protected]>
Reported-by: Nathan Ringo <[email protected]>
Based-on-patch-by: Diego Becciolini <[email protected]>
Based-on-patch-by: Sergei Utinski <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit c22effb)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit adc4fa2)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member Author

didn't backport this (yet) to the 1.1 branch, as it depends on (among others) #2324. Wasn't sure how badly we want this for 1.1, with 1.3 already being worked on

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #3161 into release/1.2 will decrease coverage by 0.01%.
The diff coverage is 25%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release/1.2    #3161      +/-   ##
===============================================
- Coverage        43.76%   43.74%   -0.02%     
===============================================
  Files              101      101              
  Lines            10738    10743       +5     
===============================================
+ Hits              4699     4700       +1     
- Misses            5309     5311       +2     
- Partials           730      732       +2
Flag Coverage Δ
#linux 47.38% <25%> (-0.02%) ⬇️
#windows 40.85% <ø> (ø) ⬆️
Impacted Files Coverage Δ
mount/mountinfo_linux.go 57.4% <25%> (-3.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d58ce1...f8d644d. Read the comment docs.

1 similar comment
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #3161 into release/1.2 will decrease coverage by 0.01%.
The diff coverage is 25%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release/1.2    #3161      +/-   ##
===============================================
- Coverage        43.76%   43.74%   -0.02%     
===============================================
  Files              101      101              
  Lines            10738    10743       +5     
===============================================
+ Hits              4699     4700       +1     
- Misses            5309     5311       +2     
- Partials           730      732       +2
Flag Coverage Δ
#linux 47.38% <25%> (-0.02%) ⬇️
#windows 40.85% <ø> (ø) ⬆️
Impacted Files Coverage Δ
mount/mountinfo_linux.go 57.4% <25%> (-3.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d58ce1...f8d644d. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit f2702c5 into containerd:release/1.2 Apr 2, 2019
@thaJeztah thaJeztah deleted the 1.2_backport_fix_parseinfofile_parsing branch April 2, 2019 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants