Skip to content

rtpmjpeg: reject reserved quantization value 127 - #1161

Merged
aler9 merged 1 commit into
bluenviron:mainfrom
winklemad:rtpmjpeg-reject-q127
Sep 9, 2026
Merged

aler9 merged 1 commit into
bluenviron:mainfrom
winklemad:rtpmjpeg-reject-q127

Conversation

@winklemad

Copy link
Copy Markdown
Contributor

RFC 2435 reserves quantization values 100-127. The header guard used Quantization > 99 && Quantization < 127, so it rejected 100-126 but let 127 through.

A leaked 127 then reaches makeQuantizationTables(127) (Quantization < 128 takes the fixed-table path), where scale = 200 - 2*127 = -54. The negative scale produces a corrupt quantization table (e.g. tables[0][0] = 248) instead of the clean rejection every other reserved value gets — a silently corrupt JPEG for a non-conforming sender.

Fix: widen the guard to < 128 so all of 100-127 are rejected. Added TestHeaderJpegUnmarshalErrors covering 0, 100, 126 and 127 (fails on 127 before the change).

RFC 2435 reserves quantization values 100-127, but the guard used
`Quantization < 127`, letting 127 through. It then reached
makeQuantizationTables(127), where scale = 200 - 2*127 = -54, producing
a corrupt quantization table instead of a clean rejection.
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.20%. Comparing base (01951b2) to head (11d7530).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1161      +/-   ##
==========================================
- Coverage   87.23%   87.20%   -0.03%     
==========================================
  Files         133      133              
  Lines       10623    10623              
==========================================
- Hits         9267     9264       -3     
- Misses       1356     1359       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@winklemad

Copy link
Copy Markdown
Contributor Author

CI note: the test_32 failure is TestServerRecordErrorSetup/invalid_transport in the server package — this PR only touches pkg/format/rtpmjpeg (whose tests pass on 32-bit), so it looks pre-existing on the 386 runner and unrelated to this change. codecov is -0.03% from the added guard branch.

@aler9
aler9 merged commit 4ed5031 into bluenviron:main Sep 9, 2026
11 of 13 checks passed
@aler9

aler9 commented Sep 9, 2026

Copy link
Copy Markdown
Member

merged, thanks

@winklemad

Copy link
Copy Markdown
Contributor Author

Thanks @aler9!

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