# HG changeset patch # User Sebastien Jodogne # Date 1597831142 -7200 # Node ID 8c559dd5034b1fa2ae8e5195ece7ab744f803a20 # Parent b56f3a37a4a1b1ce45cc0d6c44891ac46e6d5eac Fix possible crash in HttpClient if sending multipart body (can occur in STOW-RS) diff -r b56f3a37a4a1 -r 8c559dd5034b NEWS --- a/NEWS Wed Aug 19 11:18:55 2020 +0200 +++ b/NEWS Wed Aug 19 11:59:02 2020 +0200 @@ -1,10 +1,18 @@ Pending changes in the mainline =============================== +Plugins +------- + +* New config option "Worklist.LimitAnswers" for the sample modality worklist plugin + +Maintenance +----------- + * Add missing tag "Retrieve AE Title (0008,0054)" in C-FIND SCP responses * Fix DICOM SCP filters if some query tag has more than 256 characters * "/series/.../ordered-slices" supports spaces in Image Position/Orientation Patient tags -* New config option "Worklist.LimitAnswers" for the sample modality worklist plugin +* Fix possible crash in HttpClient if sending multipart body (can occur in STOW-RS) Version 1.7.2 (2020-07-08) diff -r b56f3a37a4a1 -r 8c559dd5034b OrthancFramework/Sources/ChunkedBuffer.cpp --- a/OrthancFramework/Sources/ChunkedBuffer.cpp Wed Aug 19 11:18:55 2020 +0200 +++ b/OrthancFramework/Sources/ChunkedBuffer.cpp Wed Aug 19 11:59:02 2020 +0200 @@ -101,7 +101,7 @@ // Optimization if Orthanc >= 1.7.3, to speed up in the presence of many small chunks if (pendingPos_ + chunkSize <= pendingBuffer_.size()) { - // There remain enough place in the pending buffer + // There remains enough place in the pending buffer memcpy(&pendingBuffer_[pendingPos_], chunkData, chunkSize); pendingPos_ += chunkSize; } diff -r b56f3a37a4a1 -r 8c559dd5034b OrthancFramework/Sources/HttpClient.cpp --- a/OrthancFramework/Sources/HttpClient.cpp Wed Aug 19 11:18:55 2020 +0200 +++ b/OrthancFramework/Sources/HttpClient.cpp Wed Aug 19 11:59:02 2020 +0200 @@ -197,8 +197,8 @@ { private: HttpClient::IRequestBody* body_; - std::string sourceBuffer_; - size_t sourceBufferTransmittedSize_; + std::string pending_; + size_t pendingPos_; size_t CallbackInternal(char* curlBuffer, size_t curlBufferSize) @@ -213,65 +213,63 @@ throw OrthancException(ErrorCode_InternalError); } - // Read chunks from the body stream so as to fill the target buffer - size_t curlBufferFilledSize = 0; - size_t sourceRemainingSize = sourceBuffer_.size() - sourceBufferTransmittedSize_; - bool hasMore = true; - - while (sourceRemainingSize < curlBufferSize && hasMore) + if (pendingPos_ + curlBufferSize <= pending_.size()) + { + assert(sizeof(char) == 1); + memcpy(curlBuffer, &pending_[pendingPos_], curlBufferSize); + pendingPos_ += curlBufferSize; + return curlBufferSize; + } + else { - if (sourceRemainingSize > 0) + ChunkedBuffer buffer; + buffer.SetPendingBufferSize(curlBufferSize); + + if (pendingPos_ < pending_.size()) { - // transmit the end of current source buffer - memcpy(curlBuffer + curlBufferFilledSize, - sourceBuffer_.data() + sourceBufferTransmittedSize_, sourceRemainingSize); - - curlBufferFilledSize += sourceRemainingSize; + buffer.AddChunk(&pending_[pendingPos_], pending_.size() - pendingPos_); + } + + // Read chunks from the body stream so as to fill the target buffer + std::string chunk; + + while (buffer.GetNumBytes() < curlBufferSize && + body_->ReadNextChunk(chunk)) + { + buffer.AddChunk(chunk); } - // start filling a new source buffer - sourceBufferTransmittedSize_ = 0; - sourceBuffer_.clear(); - - hasMore = body_->ReadNextChunk(sourceBuffer_); - - sourceRemainingSize = sourceBuffer_.size(); - } + buffer.Flatten(pending_); + pendingPos_ = std::min(pending_.size(), curlBufferSize); - if (sourceRemainingSize > 0 && - curlBufferSize > curlBufferFilledSize) - { - size_t s = std::min(sourceRemainingSize, curlBufferSize - curlBufferFilledSize); + if (pendingPos_ != 0) + { + memcpy(curlBuffer, pending_.c_str(), pendingPos_); + } - memcpy(curlBuffer + curlBufferFilledSize, - sourceBuffer_.data() + sourceBufferTransmittedSize_, s); - - sourceBufferTransmittedSize_ += s; - curlBufferFilledSize += s; + return pendingPos_; } - - return curlBufferFilledSize; } public: CurlRequestBody() : body_(NULL), - sourceBufferTransmittedSize_(0) + pendingPos_(0) { } void SetBody(HttpClient::IRequestBody& body) { body_ = &body; - sourceBufferTransmittedSize_ = 0; - sourceBuffer_.clear(); + pending_.clear(); + pendingPos_ = 0; } void Clear() { body_ = NULL; - sourceBufferTransmittedSize_ = 0; - sourceBuffer_.clear(); + pending_.clear(); + pendingPos_ = 0; } bool IsValid() const diff -r b56f3a37a4a1 -r 8c559dd5034b OrthancFramework/UnitTestsSources/RestApiTests.cpp --- a/OrthancFramework/UnitTestsSources/RestApiTests.cpp Wed Aug 19 11:18:55 2020 +0200 +++ b/OrthancFramework/UnitTestsSources/RestApiTests.cpp Wed Aug 19 11:59:02 2020 +0200 @@ -1034,7 +1034,7 @@ const void* bodyData, size_t bodySize) { - printf("received %llu\n", bodySize); + printf("received %lu\n", bodySize); const uint8_t* b = reinterpret_cast(bodyData); @@ -1055,7 +1055,37 @@ #include "../Sources/HttpServer/HttpServer.h" -TEST(Toto, DISABLED_Toto) +TEST(HttpClient, DISABLED_Issue156_Slow) +{ + // https://bugs.orthanc-server.com/show_bug.cgi?id=156 + + TotoServer handler; + HttpServer server; + server.SetPortNumber(5000); + server.Register(handler); + server.Start(); + + WebServiceParameters w; + w.SetUrl("http://localhost:5000"); + + // This is slow in Orthanc <= 1.5.8 (issue 156) + TotoBody body(600 * 1024 * 1024, 6 * 1024 * 1024 - 17); + + HttpClient c(w, "toto"); + c.SetMethod(HttpMethod_Post); + c.AddHeader("Expect", ""); + c.AddHeader("Transfer-Encoding", "chunked"); + c.SetBody(body); + + std::string s; + ASSERT_TRUE(c.Apply(s)); + ASSERT_EQ("ok", s); + + server.Stop(); +} + + +TEST(HttpClient, DISABLED_Issue156_Crash) { TotoServer handler; HttpServer server; @@ -1065,20 +1095,19 @@ WebServiceParameters w; w.SetUrl("http://localhost:5000"); - - //TotoBody body(600 * 1024 * 1024, 6 * 1024 * 1024 - 17); - TotoBody body(32 * 1024, 1); // This crashes Orthanc 1.6.0 to 1.7.2 + + // This crashes Orthanc 1.6.0 to 1.7.2 + TotoBody body(32 * 1024, 1); HttpClient c(w, "toto"); c.SetMethod(HttpMethod_Post); c.AddHeader("Expect", ""); c.AddHeader("Transfer-Encoding", "chunked"); c.SetBody(body); - + std::string s; ASSERT_TRUE(c.Apply(s)); - - printf(">> [%s]\n", s.c_str()); + ASSERT_EQ("ok", s); server.Stop(); }