Skip to content

Conversation

@MegaManSec
Copy link
Contributor

@MegaManSec MegaManSec commented Sep 9, 2025

Previously, we interpreted 213 SIZE by reading only the decimal
prefix, parsing leading digits and ignoring any trailing text and
accepted 0 to fix HEAD on empty files. If digits are missing, the value
overflows, or is negative, we warn and mark the size unknown (-1).

Interpret 213 SIZE as a decimal prefix. Parse leading digits and
ignore trailing text. If missing digits, overflow, or negative,
warn and mark size unknown (-1). Accept 0 to fix HEAD on empty
files. Keep restart logic safe by not using REST when size is
unknown.
@squid-anubis

This comment was marked as resolved.

@squid-anubis squid-anubis added M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels and removed M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels labels Sep 9, 2025
@yadij yadij added the S-waiting-for-author author action is expected (and usually required) label Sep 10, 2025
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current PR poorly duplicates complex httpHeaderParseOffset() code. I support improving this FTP size parser, but it is better to wait for my planned PR that implements high-quality reusable integer parser than to add one more complex variation that PR would have to deal with. I am labeling this PR accordingly.

@rousskov rousskov added S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) and removed S-waiting-for-author author action is expected (and usually required) labels Sep 10, 2025
@rousskov rousskov changed the title FTP: Allow 0-byte file sizes, and harden size parsing FTP: Allow 0-byte file sizes and harden size parsing Sep 10, 2025
@yadij yadij requested a review from rousskov December 21, 2025 09:20
@rousskov rousskov removed their request for review December 22, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants