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

IO.FS.readFile corrupts the contents of files containing null bytes #4891

Closed
2 of 3 tasks
eric-wieser opened this issue Aug 1, 2024 · 1 comment · Fixed by #4906
Closed
2 of 3 tasks

IO.FS.readFile corrupts the contents of files containing null bytes #4891

eric-wieser opened this issue Aug 1, 2024 · 1 comment · Fixed by #4906
Labels
bug Something isn't working P-high We will work on this issue

Comments

@eric-wieser
Copy link
Contributor

eric-wieser commented Aug 1, 2024

Prerequisites

Please put an X between the brackets as you perform the following steps:

Description

IO.readFile produces a string that is not contained in the original file.

Context

This came up while trying to read /proc/{pid}/cmdline, which contains null bytes but is valid UTF8.

Steps to Reproduce

  1. Run the following, adjusting path to be a temporary file of your choice
#eval do
  let path := "/tmp/lean4-4891"
  IO.FS.writeBinFile path
    ("Hello\x00(this is a very secret message that must never be shared) world".toUTF8)
  let x ← IO.FS.readFile path
  IO.println x

Expected behavior: The secret message is visible

Actual behavior: The output is Hello world. The first 64-byte chunk ends at the the ) character, and is discarded due to the null byte.

Versions

4.10.0

Additional Information

[Additional information, configuration or data that might be necessary to reproduce the issue]

Impact

Add 👍 to issues you consider important. If others are impacted by this issue, please ask them to add 👍 to it.

@eric-wieser eric-wieser added the bug Something isn't working label Aug 1, 2024
@eric-wieser
Copy link
Contributor Author

Ah, mostly a duplicate of #3546

@leanprover-bot leanprover-bot added the P-high We will work on this issue label Aug 2, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 7, 2024
This PR:
- changes the implementation of `readBinFile` and `readFile` to only
require two system calls (`stat` + `read`) instead of one `read` per
1024 byte chunk.
- fixes a bug where `Handle.getLine` would get tripped up by a NUL
character in the line and cut the string off. This is caused by the fact
that the original implementation uses `strlen` and `lean_mk_string`
which is the backer of `mk_string` does so as well.
- fixes a bug where `Handle.putStr` and thus by extension `writeFile`
would get tripped up by a NUL char in the line and cut the string off.
Cause here is the use of `fputs` when a NUL char is possible.

Closes: #4891 
Closes: #3546
Closes: #3741
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P-high We will work on this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants