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

refactor: detect large files in Io functions #9828

Merged
merged 3 commits into from
Jan 29, 2024
Merged

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented Jan 24, 2024

Io.read_all and related functions read the contents of a file in a string, which has a size limit (Sys.max_string_length) and can be an issue in 32-bit systems. This makes an explicit check and raises a Code_error in these situations.

@emillon emillon force-pushed the read-file-unless-large branch from e219720 to 81d9c1e Compare January 24, 2024 10:57
@emillon emillon requested a review from rgrinberg January 24, 2024 10:57
@emillon emillon marked this pull request as ready for review January 24, 2024 10:57
@emillon
Copy link
Collaborator Author

emillon commented Jan 24, 2024

I tried to write some tests when the channel is not seekable; in that case Buffer throws exceptions. This will probably be a lot of work to try to detect this correctly, so I instead documented this as a limitation.

`Io.read_all` and related functions read the contents of a file in a
string, which has a size limit (`Sys.max_string_length`) and can be an
issue in 32-bit systems. This makes an explicit check and raises a
`Code_error` in these situations.

Signed-off-by: Etienne Millon <[email protected]>
@emillon emillon force-pushed the read-file-unless-large branch from bdb798e to 3dc4479 Compare January 24, 2024 11:39
@emillon emillon merged commit e86d9f8 into main Jan 29, 2024
27 checks passed
@emillon emillon deleted the read-file-unless-large branch January 29, 2024 11:50
emillon added a commit to emillon/dune that referenced this pull request Feb 5, 2024
`Io.read_all` and related functions read the contents of a file in a
string, which has a size limit (`Sys.max_string_length`) and can be an
issue in 32-bit systems. This makes an explicit check and raises a
`Code_error` in these situations.

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit that referenced this pull request Feb 5, 2024
* test: add repro for #9538 (subst on 32-bit) (#9539)

Signed-off-by: Etienne Millon <[email protected]>

* refactor: detect large files in Io functions (#9828)

`Io.read_all` and related functions read the contents of a file in a
string, which has a size limit (`Sys.max_string_length`) and can be an
issue in 32-bit systems. This makes an explicit check and raises a
`Code_error` in these situations.

Signed-off-by: Etienne Millon <[email protected]>

* fix(subst): ignore large files (#9811)

Fixes #9538

This logs a warning for large files (>16MB on 32-bit systems).

Signed-off-by: Etienne Millon <[email protected]>

---------

Signed-off-by: Etienne Millon <[email protected]>
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