-
Notifications
You must be signed in to change notification settings - Fork 104
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
'clean' filter fails on small files #116
Comments
The `read -rn 8` command doesn't seem to cope with small files, and does all kinds of strange things when I try running it manually on latest macOS to test, including: - not reading anything at all from files with short first lines - reading too much from files with long first lines (more than 8 chars) Overall the `head -c8` approach used everywhere else to do this work is probably safer, and should work with small files as well.
This is great! I really appreciate you looking into this, and for your work on #114 as well! My dataset includes txz files, which are essentially zips that have been renamed. And apparently they contain null bytes in the first 8 chars, because they cause this line: I solved it by converting null bytes to spaces: It looks like all of the encryption unit tests are based on the string |
Sorry, #117
Running line 129 directly results in the same:
With this modification there are no more warnings: I am not sure why the string |
Thanks for the continued test feedback. This one is tricky. I haven't been able to reproduce the warning message on my Mac, so I Googled and it looks like that warning is new in Bash version 4.4: https://askubuntu.com/questions/926626/how-do-i-fix-warning-command-substitution-ignored-null-byte-in-input Unfortunately, while adding the simple $ firstbytes=$(head -c8 archive.txz | tr '\0' ' ')
tr: Illegal byte sequence So it looks like new bash complains about null read by The only workaround I have found so far is this hack with an explicit firstbytes=$(head -c8 archive.txz | LC_ALL=C tr -d '\0') Notice also that |
I have applied the |
So sorry for the delayed response! This fix works great for me. I'm impressed with how many variations of bash/etc that you are able to handle! There are two other instances of "head -c8" in transcrypt, do they need need this as well? |
The `read -rn 8` command doesn't seem to cope with small files, and does all kinds of strange things (on macOS at least) including: - not reading anything at all from files with short first lines - reading too much from files with long first lines (more than 8 chars) Switch to us `head -c8` instead, which is safer, plus some fiddly use of `tr` to ignore null bytes early in a way that works for older and newer versions of bash.
I'm very late getting to this, sorry. It's possible the other uses For now I'll leave the minimal change as it is, and finally get it merged to |
Awesome, thank you very much! |
# By James Murty (18) and others # Via GitHub (1) and James Murty (1) * main: (26 commits) Centralise load and save of password into functions #141 Fix date of 2.2.0 release Ensure tests use "main" as default branch name #143 Use OpenSSL for B64 encoding not `base64` which differs between Linux and Mac #140 Use core attributesFile from worktree (#137) Document `xxd` requirement, and make optional with OpenSSL < 3 (#138) Prepare for 2.2.0 release Fix when using OpenSSL 3 which no longer embeds salt in output (#135) Consolidate all git operation scripts into a single transcrypt script Fix handling of small files and files with null in first 8 bytes (#116) Improve command hint to fix secret files not encrypted in index (#120) (#130) Remove Ubuntu 16.04 LTS from test matrix (#123) Configure default Git branch name for macOS tests in GitHub Handle rename of primary branch from "master" to "main" Ensure Git index is up-to-date before dirty repo check #37 (#109) Fix incorrect salt when partially staged files are commited (#119) Use shorthand for grep options for broader compatibility (#121) Let user set a custom path to openssl #108 Install entire transcrypt script into repository Change version to indicate development "pre-release" status ... # Conflicts: # README.md # tests/_test_helper.bash # tests/test_cleanup.bats # tests/test_crypt.bats # tests/test_init.bats # tests/test_not_inited.bats # transcrypt
If you try to encrypt a small file (less than 8 chars) it fails:
The problem is with the
read
command in git_clean():transcrypt/transcrypt
Line 129 in dce1ad0
One solution is to add an
or
statement like this:The text was updated successfully, but these errors were encountered: