Skip to content

guard numeric retry-after against long overflow#3405

Open
alhudz wants to merge 1 commit into
OpenFeign:masterfrom
alhudz:retry-after-overflow-guard
Open

guard numeric retry-after against long overflow#3405
alhudz wants to merge 1 commit into
OpenFeign:masterfrom
alhudz:retry-after-overflow-guard

Conversation

@alhudz

@alhudz alhudz commented Jun 9, 2026

Copy link
Copy Markdown

Repro: a 503 whose Retry-After header is all digits but wider than long, e.g. Retry-After: 99999999999999999999.
Cause: RetryAfterDecoder.apply accepts the value with ^[0-9]+\.?0*$ then calls Long.parseLong outside the try, so the NumberFormatException escapes the error decoder instead of being ignored the way a malformed date is.
Fix: move the numeric branch into the existing try and add NumberFormatException to the catch, so an out-of-range value returns null like the date branch.

@velo velo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the focused overflow fix and regression test. The test coverage, backwards-compatibility, and security gates pass, but the required ci/circleci: pr-build check is failing, so this cannot be approved or merged until CI is green.

@alhudz

alhudz commented Jun 9, 2026

Copy link
Copy Markdown
Author

The red ci/circleci: pr-build isn't coming from this branch. The Test step dies at compile, in JavaLoggerTest and LoggerRebufferTest, with Charset cannot be converted to RequestTemplate.

Those files still call Request.create(..., Util.UTF_8), but f9159cf changed that last parameter from Charset to RequestTemplate without updating them, so the core test sources don't compile on master either. This PR is based on master (21cd9f3) and only touches ErrorDecoder.java and RetryAfterDecoderTest.java, so it just inherits the breakage and the core test reactor never gets as far as running the new test.

Happy to rebase once master compiles again, or I can fold the one-line signature fix for those two test files into this PR if you'd rather unblock it here. Let me know which you prefer.

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