Detect file reads that are longer than expected #24708
Conversation
When Bazel knows the size of a file, it can also detect whether it would go past this size while reading the file. This is virtually free due to using a buffered stream and can catch cases of concurrent modifications or even bugs in Bazel.
github-actions
bot
added
the
awaiting-review
| @@ -844,16 +844,37 @@ public static String readContent(Path inputFile, Charset charset) throws IOExcep | |||
| } | |||
|
|
|||
| /** | |||
| * Reads at most {@code limit} bytes from {@code inputFile} and returns it as a byte array. | |||
| * Reads at most {@code limit} bytes from {@code inputFile} and returns them as a byte array. | |||
| * | |||
| * @throws IOException if there was an error. | |||
| */ | |||
| public static byte[] readContentWithLimit(Path inputFile, int limit) throws IOException { | |||
There was a problem hiding this comment.
Within Bazel, this function is now only called by tests. I can get rid of it if that's also the case for Blaze.
There was a problem hiding this comment.
It's not used anywhere else internally. I guess we can get rid of it.
| @@ -844,16 +844,37 @@ public static String readContent(Path inputFile, Charset charset) throws IOExcep | |||
| } | |||
|
|
|||
| /** | |||
| * Reads at most {@code limit} bytes from {@code inputFile} and returns it as a byte array. | |||
| * Reads at most {@code limit} bytes from {@code inputFile} and returns them as a byte array. | |||
| * | |||
| * @throws IOException if there was an error. | |||
| */ | |||
| public static byte[] readContentWithLimit(Path inputFile, int limit) throws IOException { | |||
There was a problem hiding this comment.
It's not used anywhere else internally. I guess we can get rid of it.
iancha1992
added
the
team-ExternalDeps
meteorcloudy
added
team-Core
fmeum
force-pushed
the
24694-size-check
branch
from
985b05b to
b132505
Compare
| @@ -917,19 +881,27 @@ private LongReadIOException(Path path, int fileSize) { | |||
| * the number of bytes read. | |||
| * | |||
| * <p>Use this method when you already know the size of the file. The check is intended to catch | |||
| * issues where filesystems or external interference results in truncated or concurrently modified | |||
| * files. | |||
| * issues where filesystems or external interference results in truncated or concurrent | |||
There was a problem hiding this comment.
Nit [clarity]: Imo something like the following more clearly conveys the intended meaning while keeping your word choice. This is an optional suggestion, so feel free to ignore!
where the filesystem incorrectly returns truncated file contents, or where an external modification has concurrently truncated or appended to the file.
There was a problem hiding this comment.
Yes, that's clearer! Can you make the change during the import?
There was a problem hiding this comment.
Sure
@meteorcloudy fyi since I'm not sure who is going to be mailed the internal CL
There was a problem hiding this comment.
gTech will do the import, but I can suggest the edition, np!
There was a problem hiding this comment.
Or I can just edit this PR, done ;)
|
@bazel-io fork 8.0.1 |
meteorcloudy
added
awaiting-PR-merge
When Bazel knows the size of a file, it can also detect whether it would go past this size while reading the file. This is virtually free due to using a buffered stream and can catch cases of concurrent modifications or even bugs in Bazel.
Work towards #24694