Currently, http1::client::ResponseBody::try_recv yields written == 0 when there are no bytes to produce from the available input or when there is no space left in the output buffer, and there is no way for the caller to know the difference. This makes the API easy to misuse. For example, there is at least one place in the code where we run out of space in an output buffer and react to it by trying to gather more input, leading to a busyloop.
To improve the situation, this PR updates the low-level protocol logic to return a new NeedBytes condition when progress cannot be made due to the lack of input bytes. For now, the higher level client code treats this condition the same as written == 0, so there are no changes to behavior and any misuse bugs remain. In a later PR, we can change the code to actually do something different when NeedBytes happens.
There are of course other ways the API could have been hardened. For example instead of indicating the need for more input it could indicate when the output is full, such as with io::ErrorKind::WriteZero. However, one issue with that approach is currently all errors are treated as fatal due to our use of typestates, and of course making them non-fatal breaks usage of the ?-operator. So, this PR goes with the softer approach of adding another success condition, and opting for that success condition to be a need for more input rather than a need for more output as the former feels a little more natural.
Currently,
http1::client::ResponseBody::try_recvyieldswritten == 0when there are no bytes to produce from the available input or when there is no space left in the output buffer, and there is no way for the caller to know the difference. This makes the API easy to misuse. For example, there is at least one place in the code where we run out of space in an output buffer and react to it by trying to gather more input, leading to a busyloop.To improve the situation, this PR updates the low-level protocol logic to return a new
NeedBytescondition when progress cannot be made due to the lack of input bytes. For now, the higher level client code treats this condition the same aswritten == 0, so there are no changes to behavior and any misuse bugs remain. In a later PR, we can change the code to actually do something different whenNeedByteshappens.There are of course other ways the API could have been hardened. For example instead of indicating the need for more input it could indicate when the output is full, such as with
io::ErrorKind::WriteZero. However, one issue with that approach is currently all errors are treated as fatal due to our use of typestates, and of course making them non-fatal breaks usage of the?-operator. So, this PR goes with the softer approach of adding another success condition, and opting for that success condition to be a need for more input rather than a need for more output as the former feels a little more natural.