Skip to content

TUS chunk validation#10102

Merged
SpecLad merged 12 commits intodevelopfrom
gs/chunk_start_to_chunk_end
Dec 16, 2025
Merged

TUS chunk validation#10102
SpecLad merged 12 commits intodevelopfrom
gs/chunk_start_to_chunk_end

Conversation

@Grigorii777
Copy link
Contributor

Motivation and context

Problem:
The TUS upload implementation incorrectly validated chunk boundaries by checking only the start offset (chunk.offset) instead of the end offset (chunk.offset + chunk.size). This allowed clients to upload chunks that exceed the declared file size, potentially causing:

  • Buffer overflows
  • Data corruption
  • Files larger than expected
  • Violation of TUS protocol specification

Example:

  • File size: 10,000 bytes
  • Current offset: 9,000 bytes
  • Chunk size: 2,000 bytes
  • Old code: checks 9,000 > 10,000 -> passes (incorrect)
  • New code: checks 11,000 > 10,000 -> fails with 413 (correct)

Solution:

  1. Added end_offset property to TusChunk class that calculates offset + size
  2. Changed validation in mixins.py from chunk.offset > file_size to chunk.end_offset > file_size
  3. Added comprehensive test test_cannot_upload_chunk_exceeding_file_size to verify the fix

This ensures the server correctly rejects chunks whose end position would exceed the declared file size, preventing data corruption and enforcing TUS protocol compliance.

How has this been tested?

New test added:
test_cannot_upload_chunk_exceeding_file_size - Verifies that:

  1. First chunk (1000 bytes) uploads successfully
  2. Second chunk with offset=1000 and size=(file_size - 500) is rejected
  3. Server returns 413 (Request Entity Too Large)
  4. This ensures end_offset = 1000 + (file_size - 500) > file_size is properly validated

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

@Grigorii777 Grigorii777 self-assigned this Dec 11, 2025
@Grigorii777 Grigorii777 changed the title fix and test TUS chunk validation Dec 11, 2025
@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.67%. Comparing base (6b0c940) to head (10f6c4e).
⚠️ Report is 18 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #10102      +/-   ##
===========================================
+ Coverage    75.93%   82.67%   +6.73%     
===========================================
  Files          428      485      +57     
  Lines        46337    49504    +3167     
  Branches      4143     4162      +19     
===========================================
+ Hits         35186    40927    +5741     
+ Misses       11151     8577    -2574     
Components Coverage Δ
cvat-ui 77.59% <ø> (-0.05%) ⬇️
cvat-server 86.52% <100.00%> (+12.03%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud
Copy link

@SpecLad SpecLad merged commit 53184f9 into develop Dec 16, 2025
40 checks passed
@SpecLad SpecLad deleted the gs/chunk_start_to_chunk_end branch December 16, 2025 11:48
@cvat-bot cvat-bot bot mentioned this pull request Dec 18, 2025
zoe-seo pushed a commit to AutoLabeling-dotdot/AutoLabelingTool that referenced this pull request Jan 22, 2026
**Problem:**
The TUS upload implementation incorrectly validated chunk boundaries by
checking only the **start offset** (`chunk.offset`) instead of the **end
offset** (`chunk.offset + chunk.size`). This allowed clients to upload
chunks that exceed the declared file size, potentially causing:
- Buffer overflows
- Data corruption
- Files larger than expected
- Violation of TUS protocol specification

**Example:**
- File size: 10,000 bytes
- Current offset: 9,000 bytes
- Chunk size: 2,000 bytes
- Old code: checks `9,000 > 10,000` -> passes (incorrect)
- New code: checks `11,000 > 10,000` -> fails with 413 (correct)

**Solution:**
1. Added `end_offset` property to `TusChunk` class that calculates
   `offset + size`
2. Changed validation in `mixins.py` from `chunk.offset > file_size` to
   `chunk.end_offset > file_size`
3. Added comprehensive test
   `test_cannot_upload_chunk_exceeding_file_size` to verify the fix

This ensures the server correctly rejects chunks whose end position
would exceed the declared file size, preventing data corruption and
enforcing TUS protocol compliance.
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.

3 participants