containerd / ttrpc

GRPC for low-memory environments
Apache License 2.0
558 stars 80 forks source link

Block sending of oversize messages #127

Closed acclassic closed 1 month ago

acclassic commented 1 year ago

While looking to solve the containerd bug #6248 (https://github.com/containerd/containerd/issues/6248#issue-1053682568) I saw that it was already solved but the message would only be checked for the length after sending. Like @dmcgowan noted the message should not even be sent because it will be discarded anyway. I changed the code so that the send function will check for the size and not send the message in case the length is longer than the max message length. I opted for an GRPC error code 8 (RESOURCE_EXHAUSTED) over error code 3 (INVALID_ARGUMENT) because it describes the error better. I hope that this is what we were looking for and I'm open to any input.

kzys commented 1 year ago

Regarding RESOURCE_EXHAUSTED,

https://github.com/grpc/grpc-go/blob/87eb5b7502493f758e76c4d09430c0049a81a557/codes/codes.go#L92-L98

This error code will be generated by the gRPC framework in out-of-memory and server overload situations, or when a message is larger than the configured maximum size.

Seems gRPC is doing the same.

kzys commented 1 year ago

Would it replace #119?

acclassic commented 1 year ago

Would it replace #119?

Thank you for checking my code.

Yes that is correct. This replacec PR #119.

thaJeztah commented 4 months ago

@acclassic are you still working on this? Are you able to address the review comments?

klihub commented 1 month ago

Closing, PR #171 merged with implementation for sender-side reject.