huggingface / huggingface/hf_transfer

The caculation of the range stop may be incorrect in the function download_async

Open
#65 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
580
Forks
41
Avg merge
7m
Merged PRs (30d)
1

Description

### System Info

- System Configuration:
- OS: Windows 10
- CPU: Intel i7-10700
- RAM: 64GB
- Python Version: 3.11.9
- hf_transfer Version: 0.1.9

### Reproduction

I am developing a self-hosted Huggingface mirror and have encountered a mismatch between the requested and actual file sizes when downloading files with HF_HUB_ENABLE_HF_TRANSFER enabled. After investigation, I think this is due to a bug in the stop calculation in the chunked download logic.

The issue arises in the following code snippet from [src/lib.rs](https://github.com/huggingface/hf_transfer/blob/edb4239df483469c5b6b12ad366ad7dce290c1d7/src/lib.rs#L240):
```rust
let stop = std::cmp::min(start + chunk_size - 1, length);
```
In the content range of http request with the format \-\, \ and \ are integers for the start and end position (zero-indexed & inclusive), respectively.

Here, length is an exclusive index, while stop is an inclusive index. This leads to an off-by-one error when `start + chunk_size - 1` equals length. In my case, a file has a length of `8946552810`. Hf client requests a range `xxxx-8946552810`. So `start + chunk_size - 1` is also `8946552810`, then stop will be `8946552810`, which is incorrect.

Although HTTP servers are very tolerant of out-of-bounds requests, based on my observations, the Huggingface server does not return a 416 HTTP error (Range Not Satisfiable) for such out-of-range requests. Instead, it simply returns the content within the valid file size range. Nevertheless, I still believe that correct range calculation is necessary here.

### Expected behavior

The correct calculation should be:
```rust
let stop = std::cmp::min(start + chunk_size - 1, length - 1);
```
or
```rust
let exclusive_stop = std::cmp::min(start + chunk_size, length);
let stop = exclusive_stop - 1;
```

This ensures that `stop` is always within the bounds of the file.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.