Live data from Hacker News

We found a bug in the hyper HTTP library

blog.cloudflare.com

11–20 of 97 posts

Re: We found a bug in the hyper HTTP library

#11

[flagged]

It is an explicit way to discard return values; `self.poll_read(cx)?` etc. alone would warn. Or in this case, `Poll >` is unwrapped once and `Result ` is being discarded. The decision to discard `Result ` should have been intentional, albeit turned out to be not always the case.

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.

Re: We found a bug in the hyper HTTP library

#12
> 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.

https://github.com/hyperium/hyper/issues/4022

Saved you 3000 words

Re: We found a bug in the hyper HTTP library

#13
post #11

Earlier quoted context omitted.

It is an explicit way to discard return values; `self.poll_read(cx)?` etc. alone would warn. Or in this case, `Poll >` is unwrapped once and `Result ` is being discarded. The decision to discard `Result ` should have been intentional, albeit turned out to be not always the case.

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.

You're missing how rust works. The function is explicitly allowed to fail, which is why it returns a Result. 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
  }
  ...

Re: We found a bug in the hyper HTTP library

#16

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.

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

Re: We found a bug in the hyper HTTP library

#17
post #13
post #11

Earlier quoted context omitted.

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.

You're missing how rust works. The function is explicitly allowed to fail, which is why it returns a Result . 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 _ =. Basica…

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.

Re: We found a bug in the hyper HTTP library

#18
> 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.

Re: We found a bug in the hyper HTTP library

#19
post #12

> 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. https…

Reminds me of another “slow client”-related bug in gunicorn: https://github.com/benoitc/gunicorn/issues/3334
Post reply on HN