From 99e8f5c3bb3dcd3a30d439295f856ef5fbd10121 Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Sun, 17 Jun 2018 01:01:51 +0200
Subject: [PATCH] add checking message size length
---
common/cpp/include/common/networking.h | 3 +--
producer/api/include/producer/producer.h | 2 +-
.../api/include/producer/producer_error.h | 13 ++++++++++
producer/api/src/producer_impl.cpp | 24 +++++++++++++++----
producer/api/src/producer_impl.h | 2 +-
producer/api/unittests/test_producer_impl.cpp | 20 ++++++++++++++++
6 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/common/cpp/include/common/networking.h b/common/cpp/include/common/networking.h
index dbcfb2a54..77814fc98 100644
--- a/common/cpp/include/common/networking.h
+++ b/common/cpp/include/common/networking.h
@@ -33,8 +33,7 @@ struct GenericRequestHeader {
GenericRequestHeader(Opcode i_op_code = kOpcodeUnknownOp, uint64_t i_data_id = 0,
uint64_t i_data_size = 0, const std::string& i_message = ""):
op_code{i_op_code}, data_id{i_data_id}, data_size{i_data_size} {
- auto size = std::min(i_message.size() + 1, kMaxMessageSize);
- memcpy(message, i_message.c_str(), size);
+ strncpy(message, i_message.c_str(), kMaxMessageSize);
}
Opcode op_code;
uint64_t data_id;
diff --git a/producer/api/include/producer/producer.h b/producer/api/include/producer/producer.h
index c9df9727d..a8c808704 100644
--- a/producer/api/include/producer/producer.h
+++ b/producer/api/include/producer/producer.h
@@ -38,7 +38,7 @@ class Producer {
//! Enables/Disables sending logs to the central server
virtual void EnableRemoteLog(bool enable) = 0;
//! Set beamtime id which producer will use to send data
- virtual void SetBeamtimeId(std::string beamtime_id) = 0;
+ virtual Error SetBeamtimeId(std::string beamtime_id) = 0;
};
}
diff --git a/producer/api/include/producer/producer_error.h b/producer/api/include/producer/producer_error.h
index b847d055b..6f40336e8 100644
--- a/producer/api/include/producer/producer_error.h
+++ b/producer/api/include/producer/producer_error.h
@@ -9,6 +9,8 @@ enum class ProducerErrorType {
kAlreadyConnected,
kConnectionNotReady,
kFileTooLarge,
+ kFileNameTooLong,
+ kBeamtimeIdTooLong,
kFileIdAlreadyInUse,
kAuthorizationFailed,
kInternalServerError,
@@ -71,6 +73,17 @@ auto const kFileTooLarge = ProducerErrorTemplate {
"File too large", ProducerErrorType::kFileTooLarge
};
+auto const kFileNameTooLong = ProducerErrorTemplate {
+ "filename too long", ProducerErrorType::kFileNameTooLong
+};
+
+auto const kBeamtimeIdTooLong = ProducerErrorTemplate {
+ "beamtime id too long", ProducerErrorType::kBeamtimeIdTooLong
+};
+
+
+
+
auto const kFileIdAlreadyInUse = ProducerErrorTemplate {
"File already in use", ProducerErrorType::kFileIdAlreadyInUse
};
diff --git a/producer/api/src/producer_impl.cpp b/producer/api/src/producer_impl.cpp
index c5be6e885..fd845c385 100644
--- a/producer/api/src/producer_impl.cpp
+++ b/producer/api/src/producer_impl.cpp
@@ -32,24 +32,32 @@ GenericRequestHeader ProducerImpl::GenerateNextSendRequest(uint64_t file_id, siz
return request;
}
-Error CheckProducerRequest(const GenericRequestHeader header) {
- if (header.data_size > ProducerImpl::kMaxChunkSize) {
+Error CheckProducerRequest(size_t file_size,size_t filename_size) {
+ if (file_size > ProducerImpl::kMaxChunkSize) {
return ProducerErrorTemplates::kFileTooLarge.Generate();
}
+ if (filename_size > kMaxMessageSize) {
+ return ProducerErrorTemplates::kFileNameTooLong.Generate();
+ }
+
return nullptr;
}
Error ProducerImpl::Send(uint64_t file_id, const void* data, size_t file_size, std::string file_name,
RequestCallback callback) {
- auto request_header = GenerateNextSendRequest(file_id, file_size, std::move(file_name));
- auto err = CheckProducerRequest(request_header);
+ auto err = CheckProducerRequest(file_size, file_name.size());
if (err) {
+ log__->Error("error checking request - "+err->Explain());
return err;
}
+
+ auto request_header = GenerateNextSendRequest(file_id, file_size, std::move(file_name));
+
+
return request_pool__->AddRequest(std::unique_ptr<Request> {new Request{beamtime_id_, request_header, data, callback}});
}
@@ -65,8 +73,14 @@ void ProducerImpl::EnableRemoteLog(bool enable) {
log__->EnableRemoteLog(enable);
}
-void ProducerImpl::SetBeamtimeId(std::string beamtime_id) {
+Error ProducerImpl::SetBeamtimeId(std::string beamtime_id) {
+ if (beamtime_id.size() > kMaxMessageSize) {
+ log__->Error("beamtime_id is too long - "+beamtime_id);
+ return ProducerErrorTemplates::kBeamtimeIdTooLong.Generate();
+ }
+
beamtime_id_ = std::move(beamtime_id);
+ return nullptr;
}
}
\ No newline at end of file
diff --git a/producer/api/src/producer_impl.h b/producer/api/src/producer_impl.h
index e04ab9b98..e76b93c27 100644
--- a/producer/api/src/producer_impl.h
+++ b/producer/api/src/producer_impl.h
@@ -32,7 +32,7 @@ class ProducerImpl : public Producer {
RequestCallback callback) override;
AbstractLogger* log__;
std::unique_ptr<RequestPool> request_pool__;
- void SetBeamtimeId(std::string beamtime_id) override;
+ Error SetBeamtimeId(std::string beamtime_id) override;
private:
GenericRequestHeader GenerateNextSendRequest(uint64_t file_id, size_t file_size, std::string file_name);
diff --git a/producer/api/unittests/test_producer_impl.cpp b/producer/api/unittests/test_producer_impl.cpp
index 55b851815..1af9d2fb2 100644
--- a/producer/api/unittests/test_producer_impl.cpp
+++ b/producer/api/unittests/test_producer_impl.cpp
@@ -69,8 +69,18 @@ TEST_F(ProducerImplTests, SendReturnsError) {
ASSERT_THAT(err, Eq(asapo::ProducerErrorTemplates::kRequestPoolIsFull));
}
+TEST_F(ProducerImplTests, ErrorIfFileNameTooLong) {
+ std::string long_string(asapo::kMaxMessageSize+100, 'a');
+ auto err = producer.Send(1, nullptr, 1, long_string, nullptr);
+ ASSERT_THAT(err, Eq(asapo::ProducerErrorTemplates::kFileNameTooLong));
+}
+
+
TEST_F(ProducerImplTests, ErrorIfSizeTooLarge) {
+ EXPECT_CALL(mock_logger, Error(testing::HasSubstr("error checking")));
+
auto err = producer.Send(1, nullptr, asapo::ProducerImpl::kMaxChunkSize + 1, "", nullptr);
+
ASSERT_THAT(err, Eq(asapo::ProducerErrorTemplates::kFileTooLarge));
}
@@ -94,4 +104,14 @@ TEST_F(ProducerImplTests, OKSendingRequest) {
}
+TEST_F(ProducerImplTests, ErrorSettingBeamtime) {
+ std::string expected_beamtimeid(asapo::kMaxMessageSize*10,'a');
+ EXPECT_CALL(mock_logger, Error(testing::HasSubstr("too long")));
+
+ auto err = producer.SetBeamtimeId(expected_beamtimeid);
+
+ ASSERT_THAT(err, Ne(nullptr));
+}
+
+
}
--
GitLab