Skip to content

tar: When reading, open the file ourselves - #1

Closed
dag-erling wants to merge 2 commits into
KlaraSystems:masterfrom
dag-erling:des/tar-read-open
Closed

dag-erling wants to merge 2 commits into
KlaraSystems:masterfrom
dag-erling:des/tar-read-open

Conversation

@dag-erling

Copy link
Copy Markdown
Collaborator

See individual commits for details.

While working on this, I noticed that the manual page seems to suggest that -a defaults to bzip2. This does not appear to be the case. It defaults to no compression, just like I would expect. I'm not sure if the manual page is incorrect or if I'm just reading it wrong.

I didn't have the time to add tests, but ideally we should have test cases in bsdtar_test for the following:

  • these all read any supported format from stdin:
    • -t without -f
    • -t with -f ''
    • -t with -f -
    • -x without -f
    • -x with -f ''
    • -x with -f -
  • these all write an uncompressed pax archive to stdout:
    • -c without -f
    • -c with -f ''
    • -c with -f -
    • -ca without -f
    • -ca with -f ''
    • -ca with -f -

Some of these may already be covered, but I doubt they all are. This would be trivial to do with atf-sh; not so much with the current framework.

The library interprets a null or empty filename as “use stdio”, so just
use NULL for that instead of going back and forth between NULL and "-".
Observable behavior is unchanged: specifying either no filename at all,
an empty filename, or "-", all result in reading from stdin or writing
to stdout, as before.
Instead of passing a filename to libarchive, open the file ourselves,
store the file descriptor in bsdtar->fd (which is otherwise unused in
the read case), and pass that to libarchive.  This very slightly speeds
up opening (by bypassing unused logic in archive_read_open_filename())
and ensures that the file descriptor is available if we need it later.
@dag-erling dag-erling closed this Jan 14, 2026
Comment thread tar/read.c
bsdtar->bytes_per_block))
if (bsdtar->filename == NULL || *bsdtar->filename == '\0')
bsdtar->fd = STDIN_FILENO;
else if ((bsdtar->fd = open(bsdtar->filename, O_RDONLY)) < 0)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This argument to a file access function is derived from
user input (an environment variable)
and then passed to open(__path).
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.

2 participants