Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI's -D fails when the argument is not a regular file #2874

Closed
JustAnotherArchivist opened this issue Nov 23, 2021 · 2 comments
Closed

CLI's -D fails when the argument is not a regular file #2874

JustAnotherArchivist opened this issue Nov 23, 2021 · 2 comments
Assignees
Labels

Comments

@JustAnotherArchivist
Copy link

JustAnotherArchivist commented Nov 23, 2021

Describe the bug
The -D option of the CLI appears to expect a regular file and fails when Bash's process substitution feature is used.

To Reproduce
Steps to reproduce the behavior:

  1. wget https://github.com/facebook/zstd/raw/c2c6a4ab40fcc327e79d5364f9c2ab1e41e6a7f8/tests/dict-files/zero-weight-dict (or another valid dict)
  2. Bash: echo test | zstd -D <(cat zero-weight-dict) >/dev/null
  3. Get a confusing error zstd: error 32 : Dictionary file /dev/fd/63 is too large (> 33554432 bytes) when the dict is actually just 153 bytes.

Expected behavior
I would expect zstd to be able to read the dict from process substitution, FIFOs, and really anything compatible with open(2) and read(2).

Desktop (please complete the following information):

  • OS: Debian oldstable and sid
  • Version: 1.3.8+dfsg-3, 1.4.8+dfsg-3 on amd64 (binary packages from Debian, not compiled from source by myself)

Additional context
A real-world example is fetching a dictionary with curl and passing it directly to zstd, such that a pipeline like input-command | zstd -D <(curl -s https://example.org/api/get-current-dictionary) | upload is possible without writing anything to disk.

Another potential use case (which may go beyond this issue) would be to read the dict from stdin, e.g. dict-command | zstd -D - file. This would however be sort of equivalent to zstd -D <(dict-command) file with Bash.

@terrelln
Copy link
Contributor

Bug:

  • The error message is very unclear. It should specify that we don't work on regular files.

Feature request:

  • Allow non-regular files for dictionaries.

@terrelln terrelln added the release-blocking Must be done by the release label Nov 23, 2021
@15596858998
Copy link
Contributor

I think this is a problem similar to the problem of ‘#2873’, both of which are not clear about the interpretation of the error message of the unconventional dictionary. I think we can solve these two problems together.

@terrelln terrelln closed this as completed Dec 6, 2021
@terrelln terrelln reopened this Dec 6, 2021
felixhandte added a commit to felixhandte/zstd that referenced this issue Dec 6, 2021
I hadn't seen facebook#2890, so I wrote my own version. I like this approach a little
better, since it does an explicit check for a regular file, rather than
passing a magic value.

Addresses facebook#2874.
felixhandte added a commit to felixhandte/zstd that referenced this issue Dec 6, 2021
I hadn't seen facebook#2890, so I wrote my own version. I like this approach a little
better, since it does an explicit check for a regular file, rather than
passing a magic value.

Addresses facebook#2874.
felixhandte added a commit to felixhandte/zstd that referenced this issue Dec 6, 2021
I hadn't seen facebook#2890, so I wrote my own version. I like this approach a little
better, since it does an explicit check for a regular file, rather than
passing a magic value.

Addresses facebook#2874.
felixhandte added a commit to felixhandte/zstd that referenced this issue Dec 7, 2021
I hadn't seen facebook#2890, so I wrote my own version. I like this approach a little
better, since it does an explicit check for a regular file, rather than
passing a magic value.

Addresses facebook#2874.
felixhandte added a commit to felixhandte/zstd that referenced this issue Dec 8, 2021
I hadn't seen facebook#2890, so I wrote my own version. I like this approach a little
better, since it does an explicit check for a regular file, rather than
passing a magic value.

Addresses facebook#2874.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants