bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

s3/client: Don't GET object contents on out-of-bound reads

Browse Source 
If S3 readable file is used inside file input stream, the latter may
call its read methods with position that is above file size. In that
case server replies with generic http error and the fact that the range
was invalid is encoded into reply body's xml.

That's not great to catch this via wrong reply status exception and xml
parsing all the more so we can know that the read is out-of-bound in
advance.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This commit is contained in:
Pavel Emelyanov
2023-11-24 17:59:14 +03:00
parent 339182287f
commit c5d85bdf79
2 changed files with 40 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
test/boost/s3_test.cc
View File

@@ -16,6 +16,7 @@
#include <seastar/core/fstream.hh>
#include <seastar/http/exception.hh>
#include <seastar/util/closeable.hh>
#include <seastar/util/short_streams.hh>
#include "test/lib/scylla_test_case.hh"
#include "test/lib/log.hh"
#include "test/lib/random_utils.hh"
@@ -233,6 +234,30 @@ SEASTAR_THREAD_TEST_CASE(test_client_readable_file) {
BOOST_REQUIRE_EQUAL(to_sstring(std::move(buf)), sstring("67890ABC"));
}
SEASTAR_THREAD_TEST_CASE(test_client_readable_file_stream) {
const sstring name(fmt::format("/{}/teststreamobject-{}", tests::getenv_safe("S3_BUCKET_FOR_TEST"), ::getpid()));
testlog.info("Make client\n");
semaphore mem(16<<20);
auto cln = s3::client::make(tests::getenv_safe("S3_SERVER_ADDRESS_FOR_TEST"), make_minio_config(), mem);
auto close_client = deferred_close(*cln);
testlog.info("Put object {}\n", name);
sstring sample("1F2E3D4C5B6A70899807A6B5C4D3E2F1");
temporary_buffer<char> data(sample.c_str(), sample.size());
cln->put_object(name, std::move(data)).get();
auto delete_object = deferred_delete_object(cln, name);
auto f = cln->make_readable_file(name);
auto close_readable_file = deferred_close(f);
auto in = make_file_input_stream(f);
auto close_stream = deferred_close(in);
testlog.info("Check input stream read\n");
auto res = seastar::util::read_entire_stream_contiguous(in).get0();
BOOST_REQUIRE_EQUAL(res, sample);
}
SEASTAR_THREAD_TEST_CASE(test_client_put_get_tagging) {
const sstring name(fmt::format("/{}/testobject-{}",
tests::getenv_safe("S3_BUCKET_FOR_TEST"), ::getpid()));

15
utils/s3/client.cc
View File

@@ -925,12 +925,22 @@ public:
}
virtual future<size_t> read_dma(uint64_t pos, void* buffer, size_t len, io_intent*) override {
co_await maybe_update_stats();
if (pos >= _stats->size) {
co_return 0;
}
auto buf = co_await _client->get_object_contiguous(_object_name, range{ pos, len });
std::copy_n(buf.get(), buf.size(), reinterpret_cast<uint8_t*>(buffer));
co_return buf.size();
}
virtual future<size_t> read_dma(uint64_t pos, std::vector<iovec> iov, io_intent*) override {
co_await maybe_update_stats();
if (pos >= _stats->size) {
co_return 0;
}
auto buf = co_await _client->get_object_contiguous(_object_name, range{ pos, utils::iovec_len(iov) });
uint64_t off = 0;
for (auto& v : iov) {
@@ -945,6 +955,11 @@ public:
}
virtual future<temporary_buffer<uint8_t>> dma_read_bulk(uint64_t offset, size_t range_size, io_intent*) override {
co_await maybe_update_stats();
if (offset >= _stats->size) {
co_return temporary_buffer<uint8_t>();
}
auto buf = co_await _client->get_object_contiguous(_object_name, range{ offset, range_size });
co_return temporary_buffer<uint8_t>(reinterpret_cast<uint8_t*>(buf.get_write()), buf.size(), buf.release());
}