cloud_storage: check that we have serialized the whole manifest #6507
Conversation
It turns out that if the underlying buffer object throws (e.g. a bad_alloc exception), std::ostream swallows the exception and sets the "badbit". If we don't check it, this can lead to the serialized manifest being truncated and to corrupt manifests being uploaded to the cloud storage. To prevent that we check that std::ostream is in a good state after serializing manifests. More info: https://en.cppreference.com/w/cpp/io/ios_base/iostate
|
/backport v22.2.x |
|
The pull request is not merged yet. Cancelling backport... |
There was a problem hiding this comment.
It turns out that if the underlying buffer object throws (e.g. a
bad_alloc exception), std::ostream swallows the exception and sets the
"badbit". If we don't check it, this can lead to the serialized manifest
being truncated and to corrupt manifests being uploaded to the cloud
storage. To prevent that we check that std::ostream is in a good state
after serializing manifests.
oh my. good catch. should we use a different serialization method?
|
/backport v22.2.x |
|
@dotnwat I guess implementing a rapidjson stream that wraps iobuf directly is not that hard. BTW it writes json one character at a time. I hope that iobuf is okay with that and doesn't create excessive number of fragments. |
it should be efficient for this case and use existing free space in the last fragment |
mmedenjak
added
kind/bug
Cover letter
It turns out that if the underlying buffer object throws (e.g. a bad_alloc exception),
std::ostreamswallows the exception and sets the "badbit". If we don't check it, this can lead to the serialized manifest being truncated and to corrupt manifests being uploaded to the cloud storage. To prevent that we check thatstd::ostreamis in a good state after serializing manifests.More info: https://en.cppreference.com/w/cpp/io/ios_base/iostate
Backport Required
UX changes
none
Release notes
Bug fixes