clawhdf5-remote, h5rs: never allocate a length the server only claims
h5rs check URL read the whole file with one read_at(0, len), len being whatever Content-Range said. BlockCache listed every block index of the span and preallocated len bytes: a server claiming 2^62 bytes for a 10 KB file made h5rs abort (memory allocation of 35184372088832 bytes failed). - BlockCache: a read spanning more than the budget (or eight max_requests) is fetched piece by piece and not kept, its output growing only as data arrives; read_ranges falls back to that per range; prefetch is clamped to the budget. - New clawhdf5_remote::download(storage, max_bytes): refuses a claimed length above the limit (RemoteError::TooLarge) before any request, then reads in 64 MiB steps. New RemoteError::Backend for read errors. - h5rs check downloads through it, with --max-download N (default 1 GiB). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -40,7 +40,9 @@ h5rs diff local.h5 http://127.0.0.1:8000/file.h5
|
||||
```
|
||||
|
||||
A URL names the whole file (`FILE/OBJECT` suffixes are for local paths).
|
||||
`check` validates every byte, so it downloads a remote file whole first.
|
||||
`check` validates every byte, so it downloads a remote file whole first —
|
||||
up to `--max-download N` bytes (default 1 GiB), refusing a longer file
|
||||
before reading any of it.
|
||||
The output is the local file's (`tests/remote.rs` compares every
|
||||
subcommand).
|
||||
|
||||
|
||||
@@ -28,7 +28,7 @@ use crate::h5::{Error, ErrorKind, H5, Kind};
|
||||
use crate::info::{self, DsInfo};
|
||||
|
||||
pub const USAGE: &str = "\
|
||||
usage: h5rs check [--data] [-q] [--max-bytes N] FILE
|
||||
usage: h5rs check [--data] [-q] [--max-bytes N] [--max-download N] FILE
|
||||
|
||||
Validate FILE's structure: walk every object from the root group, parse
|
||||
every header message, verify the checksums of version 2+ structures
|
||||
@@ -45,6 +45,9 @@ problem is printed with the address of the structure involved.
|
||||
datasets and attributes into its global heap collection
|
||||
-q, --quiet print only the problems, not the summary
|
||||
--max-bytes N largest dataset read by --data (default 1 GiB)
|
||||
--max-download N
|
||||
largest remote (URL) file downloaded to check it
|
||||
(default 1 GiB)
|
||||
|
||||
Exit status: 0 no problems, 1 problems found, 2 usage error or file not
|
||||
found, 3 internal error.";
|
||||
@@ -124,6 +127,7 @@ pub fn run(args: &mut Args, out: &mut Out) -> std::io::Result<i32> {
|
||||
let mut read_data = false;
|
||||
let mut quiet = false;
|
||||
let mut max_bytes = None;
|
||||
let mut max_download = 1u64 << 30;
|
||||
let mut file = None;
|
||||
while let Some(a) = args.next() {
|
||||
match a.as_str() {
|
||||
@@ -133,6 +137,10 @@ pub fn run(args: &mut Args, out: &mut Out) -> std::io::Result<i32> {
|
||||
Some(n) => max_bytes = Some(n),
|
||||
None => return args.usage_error(out, "--max-bytes needs a number", USAGE),
|
||||
},
|
||||
"--max-download" => match args.number() {
|
||||
Some(n) => max_download = n,
|
||||
None => return args.usage_error(out, "--max-download needs a number", USAGE),
|
||||
},
|
||||
"-h" | "--help" => {
|
||||
writeln!(out.o, "{USAGE}")?;
|
||||
return Ok(0);
|
||||
@@ -150,7 +158,7 @@ pub fn run(args: &mut Args, out: &mut Out) -> std::io::Result<i32> {
|
||||
let path = std::path::Path::new(&file);
|
||||
let mut h5 = if crate::h5::is_url(&file) {
|
||||
// check validates every byte, so a remote file is downloaded whole.
|
||||
match H5::open_arg_whole(&file) {
|
||||
match H5::open_arg_whole(&file, max_download) {
|
||||
Ok(h) => h,
|
||||
Err(e) => {
|
||||
writeln!(out.e, "h5rs check: {e}")?;
|
||||
|
||||
@@ -248,8 +248,10 @@ impl H5 {
|
||||
|
||||
/// [`H5::open_arg`], with a remote file downloaded whole into memory
|
||||
/// first — for `check`, which validates every byte of the file anyway
|
||||
/// and parses it as one slice ([`H5::data`]).
|
||||
pub fn open_arg_whole(arg: &str) -> Result<H5> {
|
||||
/// and parses it as one slice ([`H5::data`]). A remote file longer
|
||||
/// than `max_download` bytes is refused before anything is read: its
|
||||
/// length is only what the server claims.
|
||||
pub fn open_arg_whole(arg: &str, max_download: u64) -> Result<H5> {
|
||||
let h5 = H5::open_arg(arg)?;
|
||||
if h5.file.contiguous_bytes().is_some() {
|
||||
return Ok(h5);
|
||||
@@ -259,12 +261,8 @@ impl H5 {
|
||||
let storage =
|
||||
clawhdf5_remote::storage_for_url(arg, &clawhdf5_remote::Options::default())
|
||||
.map_err(|e| Error::new(format!("{arg}: {e}")))?;
|
||||
let len = usize::try_from(storage.len())
|
||||
.map_err(|_| Error::new(format!("{arg}: too large to download")))?;
|
||||
let bytes = storage
|
||||
.read_at(0, len)
|
||||
.map_err(|e| Error::new(format!("{arg}: {e}")))?
|
||||
.into_owned();
|
||||
let bytes = clawhdf5_remote::download(&*storage, max_download)
|
||||
.map_err(|e| Error::new(format!("{arg}: {e}")))?;
|
||||
let size = bytes.len() as u64;
|
||||
let file = File::from_bytes(bytes).map_err(|e| {
|
||||
Error::new(format!("{arg}: not an HDF5 file this tool can open: {e}"))
|
||||
@@ -272,7 +270,10 @@ impl H5 {
|
||||
Ok(H5::new(PathBuf::from(arg), file, size))
|
||||
}
|
||||
#[cfg(not(feature = "remote"))]
|
||||
unreachable!("open_arg refuses URLs without the remote feature")
|
||||
{
|
||||
let _ = max_download;
|
||||
unreachable!("open_arg refuses URLs without the remote feature")
|
||||
}
|
||||
}
|
||||
|
||||
/// The file's bytes from the superblock on: what every address indexes.
|
||||
|
||||
@@ -98,3 +98,24 @@ fn url_errors_are_clean() {
|
||||
assert!(out.contains("`https` feature"), "{out}");
|
||||
}
|
||||
}
|
||||
|
||||
/// `check` downloads a remote file whole, but never trusts the length the
|
||||
/// server claims: a server claiming 2^62 bytes for a small file is refused
|
||||
/// before anything is allocated (it used to abort the process), and
|
||||
/// `--max-download` caps real files too.
|
||||
#[test]
|
||||
fn check_refuses_a_remote_file_beyond_the_download_limit() {
|
||||
use std::sync::atomic::Ordering;
|
||||
let tall = Path::new(env!("CARGO_MANIFEST_DIR")).join("../clawhdf5/tests/fixtures/tall.h5");
|
||||
let server = server::Server::start(vec![("/t.h5".into(), std::fs::read(&tall).unwrap())]);
|
||||
let url = server.url("/t.h5");
|
||||
let (out, rc) = h5rs(&["check", &url]);
|
||||
assert_eq!(rc, 0, "{out}");
|
||||
let (out, rc) = h5rs(&["check", "--max-download", "1000", &url]);
|
||||
assert_eq!(rc, 2, "{out}");
|
||||
assert!(out.contains("download limit of 1000 bytes"), "{out}");
|
||||
server.shared.fake_total.store(1 << 62, Ordering::SeqCst);
|
||||
let (out, rc) = h5rs(&["check", &url]);
|
||||
assert_eq!(rc, 2, "{out}");
|
||||
assert!(out.contains("more than the download limit"), "{out}");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user