-
Notifications
You must be signed in to change notification settings - Fork 277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix potential integer overflow in Balance #40
Conversation
An usize overflow can occur in `Balance::promote_to_ready` when `self.not_ready` has length 0. See my comment here: #39 (comment) Signed-off-by: Eliza Weisman <eliza@buoyant.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me, but I would prefer if @olix0r could take a look as well.
tower-balance/src/lib.rs
Outdated
@@ -151,7 +151,7 @@ where | |||
debug!("promoting to ready: {}", n); | |||
// Iterate through the not-ready endpoints from right to left to prevent removals | |||
// from reordering services in a way that could prevent a service from being polled. | |||
for idx in (0..n-1).rev() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tioli: i think this would be slightly clearer as:
return Ok(());
}
for idx in (0..n-1).rev() {
...
}
it makes the intent a bit clearer...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't follow? This is the line that was removed.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
The `..` syntax creates a _half-open_ range (see https://doc.rust-lang.org/std/ops/struct.Range.html), so all that messing about with `n-1` in #39 and #40 was never actually necessary. This actually fixes the Conduit test I mentioned in #39 (comment); it no longer hangs. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
An usize overflow can occur in
Balance::promote_to_ready
whenself.not_ready
has length 0.See my comment here: #39 (comment)
Signed-off-by: Eliza Weisman eliza@buoyant.io