Skip to content

fix(caddy): parse thread and request counts as int - #2722

Merged
dunglas merged 1 commit into
mainfrom
fix/caddy-int-overflow-32bit
Oct 16, 2026
Merged

dunglas merged 1 commit into
mainfrom
fix/caddy-int-overflow-32bit

Conversation

@dunglas

@dunglas dunglas commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

num_threads, max_threads, max_requests, the worker num and max_threads options, and php-server --worker file,N are parsed with strconv.ParseUint(s, 10, 32) and then converted to int. On 32-bit platforms (we ship linux/386, linux/arm/v6 and linux/arm/v7 images), values between 2^31 and 2^32-1 wrap to negative numbers: max_threads 4294967295 becomes -1, which means auto.

These values are now parsed with strconv.Atoi through a parseNonNegativeInt helper. Nothing gets converted, so nothing can wrap, and out-of-range input returns a range error on every platform. Negative values are still rejected explicitly, because the runtime treats them as sentinels (auto thread count, default worker count). 64-bit builds accept the same values as before.

Reported by CodeQL (go/incorrect-integer-conversion) while setting it up in #2720.

@dunglas dunglas changed the title fix(caddy): reject thread and request counts that overflow int on 32-bit fix(caddy): parse thread and request counts as int Oct 16, 2026
@dunglas
dunglas force-pushed the fix/caddy-int-overflow-32bit branch from 6ac3398 to 868a6b7 Compare October 16, 2026 12:22
@dunglas
dunglas marked this pull request as ready for review October 16, 2026 12:27

@AlliBalliBaba AlliBalliBaba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caddy seems to currently generally swallow error messages on startup (probably already fixed here?)

@dunglas
dunglas merged commit 609b44c into main Oct 16, 2026
41 checks passed
@dunglas
dunglas deleted the fix/caddy-int-overflow-32bit branch October 16, 2026 21:58
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