Skip to content

reject header varint field sent as section - #157

Open
zayeem06 wants to merge 1 commit into
google:masterfrom
zayeem06:header-field-wire-type
Open

reject header varint field sent as section#157
zayeem06 wants to merge 1 commit into
google:masterfrom
zayeem06:header-field-wire-type

Conversation

@zayeem06

@zayeem06 zayeem06 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

DecodeHeader rejects a width/height/version/subsampling field that arrives as a length-delimited section by calling Fail without returning, so a crafted header slips past the check and the field value is left unset in varint_values and read later on. Adding the missing return rejects the stream with BRUNSLI_INVALID_BRN, in line with every other check in the function. I also added a small decode test that feeds such a header and asserts it is rejected.

@eustas

eustas commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Please sort includes (put <brunsli/ headers at top)

@zayeem06

zayeem06 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Done, moved the brunsli headers to the top and sorted them.

@zayeem06

Copy link
Copy Markdown
Contributor Author

any update?

@eustas

eustas commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Terribly sorry, please rebase again. I've fixed pipelines just recently and added linter

eustas
eustas previously approved these changes Aug 19, 2026
@zayeem06

Copy link
Copy Markdown
Contributor Author

Rebased onto master and squashed into one commit. Ran clang-format so the includes pass the new linter, and the c_api_test still passes locally.

@zayeem06
zayeem06 force-pushed the header-field-wire-type branch from 5ac5587 to 6ff4676 Compare August 20, 2026 07:21
@zayeem06

Copy link
Copy Markdown
Contributor Author

No worries. Rebased on latest master, resolved the include conflict (brunsli headers up top, standard ones after), and c_api_test still passes locally.

@eustas

eustas commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Almost. Just a tiny formatting issue.

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