fix for archiving long paths that have path components starting with ".." crossing the 100-character mark #390
Conversation
…ith .. at 100-character mark his test checks very particular scenario where path component starting with ".." of a long path gets split at 100-byte mark so that ".." goes into header and gets interpreted as parent dir (and rejected).
This is a fix for very specific scenario where a path component starting with ".." of a long path gets split at 100-byte mark so that ".." goes into header and gets interpreted as parent dir (and rejected). See tests for repro of the issue.
tests/all.rs
Outdated
| let mut ar = Builder::new(Vec::new()); | ||
|
|
||
| let mut header = Header::new_gnu(); | ||
| header.set_cksum(); |
There was a problem hiding this comment.
calling set_cksum I think should always be the end thing, but here you can probably just drop it.
|
@cgwalters Thanks for all the comments. I have fixed the coding style issues you raised making the code hopefully easier to review for the next round. |
| @@ -768,7 +768,7 @@ fn prepare_header_path(dst: &mut dyn Write, header: &mut Header, path: &Path) -> | |||
| Ok(s) => s, | |||
| Err(e) => str::from_utf8(&data[..e.valid_up_to()]).unwrap(), | |||
| }; | |||
| header.set_path(truncated)?; | |||
| header.set_truncated_path_for_gnu_header(&truncated)?; | |||
There was a problem hiding this comment.
One thing got me uncertain; this is a GNU specific behavior but we're operating on a generic tar &mut Header here.
In looking, the code today inside the generic _set_path does
if let Some(ustar) = self.as_ustar_mut() {
return ustar.set_path(path);
}
And bypasses all of this. So I think what we have here is OK, but what I feel would improve things here is if we did a cast as_gnu_mut() and operated on that instead or so.
But anyways again, not a new issue in this code.
There was a problem hiding this comment.
but what I feel would improve things here is if we did a cast
as_gnu_mut()and operated on that instead or so
I agree that it's a good idea but unfortunately a major refactor. From what I can see the problem starts at: prepare_header_path function which acts as de-facto set_path that can handle GNU header. I think it's done this way because unlike other set_path functions (the vanilla and ustar one), this one doesn't just influence header structure but emits a new entry into archive containing that long path.
The gnu tar supports arbirary path length by putting path truncated to standard 100 chars into the header and the rest is appended to contents.
tar-rsvalidates that no path components should be exactly ".." but in this case when a component starting with ".." (for example file named "..some_file") gets truncated after 2 characters we hit this validation and can't tar such file.I have verified that gnu
tarcommand can handle such paths and actually puts truncated..in the header. This pull request makestar-rsbehave the same.See tests for repro of the issue.
(I think the code for handling this special case is quite ugly - I'd appreciate suggestions for making it better)