So “fearless concurrency” still only happens when one just decides to not be afraid… :)
Twey 22 minutes ago [-]
This would have been flagged by Clippy lints `let_underscore_untyped` or `let_underscore_must_use`, which sadly are not enabled by default.
worldsavior 1 hours ago [-]
> We spent six weeks chasing a nearly invisible bug — a race condition that occurred only under specific conditions — in the hyper library that impacted how the Images binding returned processed image data back to the client. In the end, it took four lines of code to fix it.
That's a long time, must be frustrating.
edelbitter 4 hours ago [-]
Cloudflare does not notice (until a customer complains) that they are sending broken responses at scale? I would have thought they would notice this from sampling and linting a few replies.. just in case they did something like Cloudbleed again.
24 minutes ago [-]
ramon156 1 hours ago [-]
Can you get reasonable results without exposing sensitive info? I'm asking because I genuinely have no idea what it's like at their scale
Thaxll 24 minutes ago [-]
So much for Rust forcing you to handle errors.
wongarsu 13 minutes ago [-]
You could argue the bug happened exactly because hyper's poll_flush treats flushing some but not all data as a successful return, not an error case.
atoav 17 minutes ago [-]
You could say the exact same thing about safety belts and airbags in cars after someone has died in a crash.
Why even bother with measures that prevent many problems if they won't prevent all of them, right?
microgpt 50 minutes ago [-]
Would using Rust have prevented this?
re-thc 23 minutes ago [-]
Isn't this already Rust?
pjmlp 9 minutes ago [-]
That was obviously a joke question, pointing that Rust isn't the solution for everything.
Cthulhu_ 24 minutes ago [-]
The Hyper library in question is a Rust library.
Did you read the article, or are you a "use rust" parrot / bot based on titles?
waysa 9 minutes ago [-]
Sarcasm. (I guess)
algoth1 2 hours ago [-]
I wonder if this bug was found via project glasswing
re-thc 21 minutes ago [-]
> I wonder if this bug was found via project glasswing
Did you read how they said it took weeks? Would run out of tokens at that rate...
100ms 3 hours ago [-]
> The failure was caused by a timing-dependent race condition in hyper’s HTTP/1 connection handling. When the reader was slower and the socket buffer filled, poll_flush returned Poll::Pending, but the dispatch loop discarded that result. Hyper then treated the response as complete and shut down the socket while data remained buffered internally, causing the client to receive an EOF before the full body arrived.
This is the Rust idiom for “I am intentionally ignoring this return value”. The linter would have caught
self.poll_read()?;
and in fact one of the options the linter itself suggests in this case is exactly this “let underscore equals” idiom. (Arguably, this code exists because of the linter, not due to its absence!)
In any case, the return value is being “handled” - the question mark examines the result and breaks the loop if the result is not `Ok(…)`, ie if the call is not successful.
Intentionally ignoring the successful return value isn’t necessarily terrible, either - you could be calling the function for its side effect, and you don’t care what the specific result of that effect is, just as long as there is some effect. E.g. maybe you have a state machine, and this is the code that repeatedly drives it.
(Not coincidentally, polling is what you do to Futures, and Futures are state machines that you need to repeatedly drive…)
In conclusion, I do not think this is prima facie terrible code, nor is it an obvious bug. Async rust is subtle and complicated, and not always fully understood by those who nevertheless have to use it.
lifthrasiir 4 hours ago [-]
It is an explicit way to discard return values; `self.poll_read(cx)?` etc. alone would warn. Or in this case, `Poll<Result<(), Error>>` is unwrapped once and `Result<(), Error>` is being discarded. The decision to discard `Result<(), Error>` should have been intentional, albeit turned out to be not always the case.
watt 3 hours ago [-]
If they're not going to handle the return values, they should change the function signature to reflect this aspirational contract, that that function "never fails".
I see in the article they did change the poll_flush to run just-in-time at poll_shutdown. So they definitely can make a "best effort" poll_flush version that just does not return any errors for use in that loop.
But all in all? Amateur hour.
a_cul 3 hours ago [-]
You're missing how rust works. The function is explicitly allowed to fail, which is why it returns a Result<(), Error>. They're using the function calls within for their side effects. The ? at the end of each line signals that the function will short-circuit return with an error if the function call fails, and only if it is successful it returns the actual value: they just don't care about this value, hence the let _ =. Basically, they are doing the equivalent of:
let _, err = function_call();
if err {
return err
}
...
watt 1 hours ago [-]
What I am saying, is make another version of the function, which is explicitly not allowed to fail, if you want to use it in the loop.
QuantumNomad_ 4 hours ago [-]
Assigning to _ in Rust specifically means that you intentionally want to discard the value, and the clippy linter and the Rust compiler both know that.
4 hours ago [-]
giammbo 4 hours ago [-]
[flagged]
r_lee 2 hours ago [-]
LLM?
Rendered at 12:01:52 GMT+0000 (Coordinated Universal Time) with Vercel.
That's a long time, must be frustrating.
Why even bother with measures that prevent many problems if they won't prevent all of them, right?
Did you read the article, or are you a "use rust" parrot / bot based on titles?
Did you read how they said it took weeks? Would run out of tokens at that rate...
https://github.com/hyperium/hyper/issues/4022
Saved you 3000 words
https://datatracker.ietf.org/doc/html/rfc9112#section-9.6 (this was already in https://datatracker.ietf.org/doc/html/rfc7230#section-6.6)
In any case, the return value is being “handled” - the question mark examines the result and breaks the loop if the result is not `Ok(…)`, ie if the call is not successful.
Intentionally ignoring the successful return value isn’t necessarily terrible, either - you could be calling the function for its side effect, and you don’t care what the specific result of that effect is, just as long as there is some effect. E.g. maybe you have a state machine, and this is the code that repeatedly drives it.
(Not coincidentally, polling is what you do to Futures, and Futures are state machines that you need to repeatedly drive…)
In conclusion, I do not think this is prima facie terrible code, nor is it an obvious bug. Async rust is subtle and complicated, and not always fully understood by those who nevertheless have to use it.
I see in the article they did change the poll_flush to run just-in-time at poll_shutdown. So they definitely can make a "best effort" poll_flush version that just does not return any errors for use in that loop.
But all in all? Amateur hour.