diff --git a/common/cpp/include/common/networking.h b/common/cpp/include/common/networking.h index dbcfb2a544e0aeaee1ffea67cf704f55e54e4e5c..77814fc9831fa3df423749b0cfedad58e1d8b042 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 c9df9727dcda440092f12992a65c247a45602dbc..a8c80870476f5ecb00ded90a075bd158c3f08cbb 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 b847d055ba27c9461f5bed4587e6869438d9827b..6f40336e81fed1fa07ac4b2380b1179626c1f1cb 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 c5be6e8852cfc037b8ab3e3832237c31be843840..fd845c3851ba5e06adad2287c23441249d7a06b0 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 e04ab9b98a81006aeef456296db243f966d45209..e76b93c27b65eb8a3f8fb5595c1a78bbe08a4162 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 55b851815d0a8fac86946ee273398c8cb6ab9817..1af9d2fb2e29a8f707fb0e9e2c7f02c2aba91dbc 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)); +} + + }