Skip to content

Commit

Permalink
Add a method to write a link name without canonicalization
Browse files Browse the repository at this point in the history
In https://github.com/ostreedev/ostree we generate a cryptographic
checksum over files and symlinks, and directories.

ostree does not currently perform any canonicalization on symlinks;
we'll respect and honor whatever bytes we're provided as input,
and replicate that on the target.

I'm using this crate to do tar serialization, which has so far worked
fine...except, I hit this corner case:

```
[root@cosa-devsh ~]# rpm -qf /usr/lib/systemd/systemd-sysv-install
chkconfig-1.13-2.el8.x86_64
[root@cosa-devsh ~]# ll /usr/lib/systemd/systemd-sysv-install
lrwxrwxrwx. 2 root root 24 Nov 29 18:08 /usr/lib/systemd/systemd-sysv-install -> ../../..//sbin/chkconfig
[root@cosa-devsh ~]#
```

But, using `set_link_name` to write the tarball, we end up with
the canonicalized path `../../../sbin/chkconfig` - i.e. without the
double `//`.  This breaks the checksum.

Now, I am a bit tempted to change ostree to do canonicalization.  But
even if we did, I'd need to *exactly* match what this crate is doing.

In the end, what I think is useful here is a method which skips
the canonicalization and internal error checking and just writes
out into the tar stream what's requested.

(I may of course also try to change the rhel8 systemd package, but
 that's going to take a while to propagate and this corner case isn't
 the only one I'm sure)
  • Loading branch information
cgwalters committed Dec 14, 2021
1 parent ec5edf1 commit 0c15a30
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/header.rs
Expand Up @@ -435,6 +435,18 @@ impl Header {
})
}

/// Sets the link name for this header without any transformation.
///
/// This function is like [`Self::set_link_name`] but accepts an arbitrary byte array.
/// Hence it will not perform any canonicalization, such as replacing duplicate `//` with `/`.
pub fn set_link_name_literal<P: AsRef<[u8]>>(&mut self, p: P) -> io::Result<()> {
self._set_link_name_literal(p.as_ref())
}

fn _set_link_name_literal(&mut self, bytes: &[u8]) -> io::Result<()> {
copy_into(&mut self.as_old_mut().linkname, bytes)
}

/// Returns the mode bits for this file
///
/// May return an error if the field is corrupted.
Expand Down
22 changes: 22 additions & 0 deletions tests/all.rs
Expand Up @@ -935,6 +935,28 @@ fn long_linkname_gnu() {
}
}

#[test]
fn linkname_literal() {
for t in [tar::EntryType::Symlink, tar::EntryType::Link] {
let mut b = Builder::new(Vec::<u8>::new());
let mut h = Header::new_gnu();
h.set_entry_type(t);
h.set_size(0);
let path = "usr/lib/systemd/systemd-sysv-install";
let target = "../../..//sbin/chkconfig";
h.set_link_name_literal(target).unwrap();
t!(b.append_data(&mut h, path, std::io::empty()));

let contents = t!(b.into_inner());
let mut a = Archive::new(&contents[..]);

let e = &t!(t!(a.entries()).next().unwrap());
assert_eq!(e.header().entry_type(), t);
assert_eq!(e.path().unwrap().to_str().unwrap(), path);
assert_eq!(e.link_name().unwrap().unwrap().to_str().unwrap(), target);
}
}

#[test]
fn encoded_long_name_has_trailing_nul() {
let td = t!(TempBuilder::new().prefix("tar-rs").tempdir());
Expand Down

0 comments on commit 0c15a30

Please sign in to comment.