common/Util.hpp | 23 ++++ net/Socket.cpp | 135 +++++++++++++++++++++++++++- net/Socket.hpp | 26 +++++ net/WebSocketHandler.hpp | 8 - test/Makefile.am | 6 - test/UnitHTTP.cpp | 224 +++++++++++++++++++++++++++++++++++++++++++++++ test/helpers.hpp | 23 ++++ wsd/DocumentBroker.cpp | 22 +++- wsd/DocumentBroker.hpp | 3 wsd/LOOLWSD.cpp | 33 +++++- 10 files changed, 472 insertions(+), 31 deletions(-)
New commits: commit 788091ae154b7a20fba6e37e119fa03c0b3bf9a0 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Wed May 22 10:54:36 2019 +0100 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Thu May 23 12:37:03 2019 +0100 tests for chunked transfer encoding parser. Change-Id: Ic55669ab7cc55bb44e8f7a00f30231b44f10535a diff --git a/test/UnitHTTP.cpp b/test/UnitHTTP.cpp index d0530fe10..e86593f22 100644 --- a/test/UnitHTTP.cpp +++ b/test/UnitHTTP.cpp @@ -13,6 +13,7 @@ #include <helpers.hpp> #include <Poco/Util/Application.h> +#include <Poco/Net/StreamSocket.h> #include <Poco/Net/StringPartSource.h> #include <Poco/Net/HTMLForm.h> #include <Poco/Net/HTTPRequest.h> @@ -37,9 +38,9 @@ public: config.setBool("ssl.enable", true); } - // FIXME: can hook with (UnitWSD::get().handleHttpRequest(request, message, socket)) ... - void invokeTest() override + void testContinue() { + std::cerr << "testContinue\n"; for (int i = 0; i < 3; ++i) { std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(Poco::URI(helpers::getTestServerURI()))); @@ -81,9 +82,135 @@ public: return; } } - // Give those convertors time to save and cleanup. - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + } + + void writeString(const std::shared_ptr<Poco::Net::StreamSocket> &socket, std::string str) + { + socket->sendBytes(str.c_str(), str.size()); + } + + bool expectString(const std::shared_ptr<Poco::Net::StreamSocket> &socket, std::string str) + { + char buffer[str.size() + 64] = { 0, }; + int got = socket->receiveBytes(buffer, str.size()); + if (got != (int)str.size() || + strncmp(buffer, str.c_str(), got)) + { + std::cerr << "testChunks got " << got << " mismatching strings '" << buffer << " vs. expected '" << str << "'\n"; + exitTest(TestResult::Failed); + return false; + } + else + return true; + } + + void testChunks() + { + std::cerr << "testChunks\n"; + + std::shared_ptr<Poco::Net::StreamSocket> socket = helpers::createRawSocket(); + + writeString( + socket, + "POST /lool/convert-to/txt HTTP/1.1\r\n" + "Host: localhost:9980\r\n" + "User-Agent: looltests/1.2.3\r\n" + "Accept: */*\r\n" + "Expect: 100-continue\r\n" + "Transfer-Encoding: chunked\r\n" + "Content-Type: multipart/form-data; " + "boundary=------------------------5a0cd5c881663db4\r\n\r\n"); + if (!expectString( + socket, + "HTTP/1.1 100 Continue\r\n\r\n")) + return; + +#define START_CHUNK_HEX(len) len "\r\n" +#define END_CHUNK "\r\n" + writeString( + socket, + START_CHUNK_HEX("8A") + "--------------------------5a0cd5c881663db4\r\n" + "Content-Disposition: form-data; name=\"data\"; filename=\"test.txt\"\r\n" + "Content-Type: text/plain\r\n" + "\r\n" + END_CHUNK + + START_CHUNK_HEX("12") + "This is some text." + END_CHUNK + + START_CHUNK_HEX("1") + "\n" + END_CHUNK + + " 4 room:for expansion!! cf. leading spaces and nasies <>!\"\'?=)\r\n" + "And " + END_CHUNK + + START_CHUNK_HEX("1") + "s" + END_CHUNK + + START_CHUNK_HEX("a") + "ome more.\n" + END_CHUNK + ); + writeString( + socket, + START_CHUNK_HEX("30") + "\r\n" + "--------------------------5a0cd5c881663db4--\r\n" + END_CHUNK); + + writeString(socket, START_CHUNK_HEX("0")); + + char buffer[4096] = { 0, }; + int got = socket->receiveBytes(buffer, 4096); + std::string start = + "HTTP/1.0 200 OK\r\n" + "Content-Disposition: attachment; filename=\"test.txt\"\r\n"; + + if (strncmp(buffer, start.c_str(), start.size())) + { + std::cerr << "missing pre-amble " << got << " '" << buffer << " vs. expected '" << start << "'\n"; + exitTest(TestResult::Failed); + return; + } + // TODO: check content-length etc. + + const char *ptr = strstr(buffer, "\r\n\r\n"); + if (!ptr) + { + std::cerr << "missing separator " << got << " '" << buffer << "\n"; + exitTest(TestResult::Failed); + return; + } + + // Oddly we need another read to get the content. + got = socket->receiveBytes(buffer, 4096); + if (got >=0 ) + buffer[got] = '\0'; + else + { + std::cerr << "No content returned " << got << "\n"; + exitTest(TestResult::Failed); + return; + } + + if (strcmp(buffer, "\357\273\277This is some text.\nAnd some more.\n")) + { + std::cerr << "unexpected file content " << got << " '" << buffer << "\n"; + exitTest(TestResult::Failed); + return; + } + } + + void invokeTest() override + { + testChunks(); + testContinue(); std::cerr << "All tests passed.\n"; exitTest(TestResult::Ok); } diff --git a/test/helpers.hpp b/test/helpers.hpp index 0ee9f451d..df43c812a 100644 --- a/test/helpers.hpp +++ b/test/helpers.hpp @@ -21,6 +21,8 @@ #include <Poco/Net/HTTPResponse.h> #include <Poco/Net/HTTPSClientSession.h> #include <Poco/Net/NetException.h> +#include <Poco/Net/StreamSocket.h> +#include <Poco/Net/SecureStreamSocket.h> #include <Poco/Net/Socket.h> #include <Poco/Path.h> #include <Poco/StringTokenizer.h> @@ -175,18 +177,33 @@ Poco::Net::HTTPClientSession* createSession(const Poco::URI& uri) #endif } -inline -std::string const & getTestServerURI() +inline int getClientPort() { static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT"); + return clientPort? atoi(clientPort) : DEFAULT_CLIENT_PORT_NUMBER; +} + +inline std::shared_ptr<Poco::Net::StreamSocket> createRawSocket() +{ + return +#if ENABLE_SSL + std::make_shared<Poco::Net::SecureStreamSocket> +#else + std::make_shared<Poco::Net::StreamSocket> +#endif + (Poco::Net::SocketAddress("127.0.0.1", getClientPort())); +} +inline +std::string const & getTestServerURI() +{ static std::string serverURI( #if ENABLE_SSL "https://127.0.0.1:" #else "http://127.0.0.1:" #endif - + (clientPort? std::string(clientPort) : std::to_string(DEFAULT_CLIENT_PORT_NUMBER))); + + std::to_string(getClientPort())); return serverURI; } commit d51815900ee03adb8eabb8b6bee24861f3e7d9bc Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Wed May 22 03:21:50 2019 +0100 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Thu May 23 12:37:03 2019 +0100 Initial chunked transfer encoding. Important for convert-to on larger documents and/or with newer curls. Change-Id: Id18be6d22741a3af7cee39a069c509e4f662977b diff --git a/common/Util.hpp b/common/Util.hpp index 96e61d4e9..9021b7ed1 100644 --- a/common/Util.hpp +++ b/common/Util.hpp @@ -165,6 +165,18 @@ namespace Util // Extract all json entries into a map. std::map<std::string, std::string> JsonToMap(const std::string& jsonString); + inline int hexDigitFromChar(char c) + { + if (c >= '0' && c <= '9') + return c - '0'; + else if (c >= 'a' && c <= 'f') + return c - 'a' + 10; + else if (c >= 'A' && c <= 'F') + return c - 'A' + 10; + else + return -1; + } + /// Dump a lineof data as hex inline std::string stringifyHexLine( const std::vector<char> &buffer, @@ -233,6 +245,17 @@ namespace Util os.flush(); } + inline std::string dumpHex (const char *legend, const char *prefix, + const std::vector<char>::iterator &startIt, + const std::vector<char>::iterator &endIt, + bool skipDup = true, const unsigned int width = 32) + { + std::ostringstream oss; + std::vector<char> data(startIt, endIt); + dumpHex(oss, legend, prefix, data, skipDup, width); + return oss.str(); + } + /// Trim spaces from the left. Just spaces. inline std::string& ltrim(std::string& s) { diff --git a/net/Socket.cpp b/net/Socket.cpp index bdc766c82..bd592025e 100644 --- a/net/Socket.cpp +++ b/net/Socket.cpp @@ -627,14 +627,21 @@ std::string LocalServerSocket::bind() return ""; } +// For a verbose life, tweak here: +#if 0 +# define LOG_CHUNK(X) LOG_TRC(X) +#else +# define LOG_CHUNK(X) +#endif + bool StreamSocket::parseHeader(const char *clientName, Poco::MemoryInputStream &message, Poco::Net::HTTPRequest &request, - size_t *requestSize) + MessageMap *map) { LOG_TRC("#" << getFD() << " handling incoming " << _inBuffer.size() << " bytes."); - assert(!requestSize || *requestSize == 0); + assert(!map || (map->_headerSize == 0 && map->_messageSize == 0)); // Find the end of the header, if any. static const std::string marker("\r\n\r\n"); @@ -648,8 +655,11 @@ bool StreamSocket::parseHeader(const char *clientName, // Skip the marker. itBody += marker.size(); - if (requestSize) - *requestSize = static_cast<size_t>(itBody - _inBuffer.begin()); + if (map) // a reasonable guess so far + { + map->_headerSize = static_cast<size_t>(itBody - _inBuffer.begin()); + map->_messageSize = map->_headerSize; + } try { @@ -680,6 +690,8 @@ bool StreamSocket::parseHeader(const char *clientName, LOG_DBG("Not enough content yet: ContentLength: " << contentLength << ", available: " << available); return false; } + if (map) + map->_messageSize += contentLength; if (request.getExpectContinue() && !_sentHTTPContinue) { @@ -689,6 +701,79 @@ bool StreamSocket::parseHeader(const char *clientName, sizeof("HTTP/1.1 100 Continue\r\n\r\n") - 1); _sentHTTPContinue = true; } + + if (request.getChunkedTransferEncoding()) + { + // keep the header + if (map) + map->_spans.push_back(std::pair<size_t, size_t>(0, itBody - _inBuffer.begin())); + + int chunk = 0; + while (itBody != _inBuffer.end()) + { + auto chunkStart = itBody; + + // skip whitespace + for (; itBody != _inBuffer.end() && isascii(*itBody) && isspace(*itBody); ++itBody) + ; // skip. + + // each chunk is preceeded by its length in hex. + size_t chunkLen = 0; + for (; itBody != _inBuffer.end(); ++itBody) + { + int digit = Util::hexDigitFromChar(*itBody); + if (digit >= 0) + chunkLen = chunkLen * 16 + digit; + else + break; + } + + LOG_CHUNK("Chunk of length " << chunkLen); + + for (; itBody != _inBuffer.end() && *itBody != '\n'; ++itBody) + ; // skip to end of line + + if (itBody != _inBuffer.end()) + itBody++; /* \n */; + + // skip the chunk. + auto chunkOffset = itBody - _inBuffer.begin(); + auto chunkAvailable = _inBuffer.size() - chunkOffset; + + if (chunkLen == 0) // we're complete. + { + map->_messageSize = chunkOffset; + return true; + } + + if (chunkLen > chunkAvailable + 2) + { + LOG_DBG("Not enough content yet in chunk " << chunk << + " starting at offset " << (chunkStart - _inBuffer.begin()) << + " chunk len: " << chunkLen << ", available: " << chunkAvailable); + return false; + } + itBody += chunkLen; + + map->_spans.push_back(std::pair<size_t,size_t>(chunkOffset, chunkLen)); + + if (*itBody != '\r' || *(itBody + 1) != '\n') + { + LOG_ERR("Missing \\r\\n at end of chunk " << chunk << " of length " << chunkLen); + LOG_CHUNK("Chunk " << chunk << " is: \n" << Util::dumpHex("", "", chunkStart, itBody + 1, false)); + return false; // TODO: throw something sensible in this case + } + else + { + LOG_CHUNK("Chunk " << chunk << " is: \n" << Util::dumpHex("", "", chunkStart, itBody + 1, false)); + } + + itBody+=2; + chunk++; + } + LOG_TRC("Not enough chunks yet, so far " << chunk << " chunks of total length " << (itBody - _inBuffer.begin())); + return false; + } } catch (const Poco::Exception& exc) { @@ -708,6 +793,39 @@ bool StreamSocket::parseHeader(const char *clientName, return true; } +bool StreamSocket::compactChunks(MessageMap *map) +{ + assert (map); + if (!map->_spans.size()) + return false; // single message. + + LOG_CHUNK("Pre-compact " << map->_spans.size() << " chunks: \n" << + Util::dumpHex("", "", _inBuffer.begin(), _inBuffer.end(), false)); + + char *first = &_inBuffer[0]; + char *dest = first; + for (auto &span : map->_spans) + { + std::memmove(dest, &_inBuffer[span.first], span.second); + dest += span.second; + } + + // Erase the duplicate bits. + size_t newEnd = dest - first; + size_t gap = map->_messageSize - newEnd; + _inBuffer.erase(_inBuffer.begin() + newEnd, _inBuffer.begin() + map->_messageSize); + + LOG_CHUNK("Post-compact with erase of " << newEnd << " to " << map->_messageSize << " giving: \n" << + Util::dumpHex("", "", _inBuffer.begin(), _inBuffer.end(), false)); + + // shrink our size to fit + map->_messageSize -= gap; + + dumpState(std::cerr); + + return true; +} + namespace HttpHelper { void sendUncompressedFileContent(const std::shared_ptr<StreamSocket>& socket, diff --git a/net/Socket.hpp b/net/Socket.hpp index 7d50495ce..a46cb5e24 100644 --- a/net/Socket.hpp +++ b/net/Socket.hpp @@ -949,9 +949,21 @@ public: return socket; } + /// Messages can be in chunks, only parts of message being valid. + struct MessageMap { + MessageMap() : _headerSize(0), _messageSize(0) {} + /// Size of HTTP headers + size_t _headerSize; + /// Entire size of data associated with this message + size_t _messageSize; + // offset + lengths to collate into the real stream + std::vector<std::pair<size_t, size_t>> _spans; + }; + /// Remove the first @count bytes from input buffer - void eraseFirstInputBytes(size_t count) + void eraseFirstInputBytes(const MessageMap &map) { + size_t count = map._headerSize; size_t toErase = std::min(count, _inBuffer.size()); if (toErase < count) LOG_ERR("#" << getFD() << ": attempted to remove: " << count << " which is > size: " << _inBuffer.size() << " clamped to " << toErase); @@ -959,12 +971,16 @@ public: _inBuffer.erase(_inBuffer.begin(), _inBuffer.begin() + count); } + /// Compacts chunk headers away leaving just the data we want + /// returns true if we did any re-sizing/movement of _inBuffer. + bool compactChunks(MessageMap *map); + /// Detects if we have an HTTP header in the provided message and /// populates a request for that. bool parseHeader(const char *clientLoggingName, Poco::MemoryInputStream &message, Poco::Net::HTTPRequest &request, - size_t *requestSize = nullptr); + MessageMap *map = nullptr); /// Get input/output statistics on this stream void getIOStats(uint64_t &sent, uint64_t &recv) @@ -985,6 +1001,8 @@ public: protected: + std::vector<std::pair<size_t, size_t>> findChunks(Poco::Net::HTTPRequest &request); + /// Called when a polling event is received. /// @events is the mask of events that triggered the wake. void handlePoll(SocketDisposition &disposition, diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp index b4c14ede6..843ca6b37 100644 --- a/net/WebSocketHandler.hpp +++ b/net/WebSocketHandler.hpp @@ -683,7 +683,7 @@ protected: LOG_TRC("Incoming client websocket upgrade response: " << std::string(&socket->getInBuffer()[0], socket->getInBuffer().size())); bool bOk = false; - size_t responseSize = 0; + StreamSocket::MessageMap map; try { @@ -699,7 +699,7 @@ protected: marker.begin(), marker.end()); if (itBody != socket->getInBuffer().end()) - responseSize = itBody - socket->getInBuffer().begin() + marker.size(); + map._headerSize = itBody - socket->getInBuffer().begin() + marker.size(); } if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_SWITCHING_PROTOCOLS @@ -729,7 +729,7 @@ protected: LOG_DBG("handleClientUpgrade exception caught."); } - if (!bOk || responseSize == 0) + if (!bOk || map._headerSize == 0) { LOG_ERR("Bad websocker server response."); @@ -738,7 +738,7 @@ protected: } setWebSocket(); - socket->eraseFirstInputBytes(responseSize); + socket->eraseFirstInputBytes(map); } #endif diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index ce16684f8..7a32429b3 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -1846,9 +1846,7 @@ private: try { #if !MOBILEAPP - size_t requestSize = 0; - - if (!socket->parseHeader("Prisoner", message, request, &requestSize)) + if (!socket->parseHeader("Prisoner", message, request)) return; LOG_TRC("Child connection with URI [" << LOOLWSD::anonymizeUrl(request.getURI()) << "]."); @@ -2061,16 +2059,23 @@ private: return; } - Poco::MemoryInputStream message(&socket->getInBuffer()[0], - socket->getInBuffer().size());; + Poco::MemoryInputStream startmessage(&socket->getInBuffer()[0], + socket->getInBuffer().size());; Poco::Net::HTTPRequest request; - size_t requestSize = 0; - if (!socket->parseHeader("Client", message, request, &requestSize)) + StreamSocket::MessageMap map; + if (!socket->parseHeader("Client", startmessage, request, &map)) return; try { + // We may need to re-write the chunks moving the inBuffer. + socket->compactChunks(&map); + Poco::MemoryInputStream message(&socket->getInBuffer()[0], + socket->getInBuffer().size()); + // update the read cursor - headers are not altered by chunks. + message.seekg(startmessage.tellg(), std::ios::beg); + // Check and remove the ServiceRoot from the request.getURI() if (!Util::startsWith(request.getURI(), LOOLWSD::ServiceRoot)) throw BadRequestException("The request does not start with prefix: " + LOOLWSD::ServiceRoot); @@ -2181,7 +2186,7 @@ private: // if we succeeded - remove the request from our input buffer // we expect one request per socket - socket->eraseFirstInputBytes(requestSize); + socket->eraseFirstInputBytes(map); #else Poco::Net::HTTPRequest request; handleClientWsUpgrade(request, std::string(socket->getInBuffer().data(), socket->getInBuffer().size()), disposition); @@ -2346,7 +2351,8 @@ private: return "application/octet-stream"; } - void handlePostRequest(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message, + void handlePostRequest(const Poco::Net::HTTPRequest& request, + Poco::MemoryInputStream& message, SocketDisposition &disposition) { LOG_INF("Post request: [" << LOOLWSD::anonymizeUrl(request.getURI()) << "]"); commit bf19e282ceba2c1814b51e305b3d3bead18bb195 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Wed May 22 02:54:12 2019 +0100 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Thu May 23 12:37:02 2019 +0100 Initial HTTP Expect: 100-continue implementation. Change-Id: Ic9aa59cac5103151d91f6eb59d12313e545c7916 diff --git a/net/Socket.cpp b/net/Socket.cpp index dbab15d08..bdc766c82 100644 --- a/net/Socket.cpp +++ b/net/Socket.cpp @@ -680,6 +680,15 @@ bool StreamSocket::parseHeader(const char *clientName, LOG_DBG("Not enough content yet: ContentLength: " << contentLength << ", available: " << available); return false; } + + if (request.getExpectContinue() && !_sentHTTPContinue) + { + LOG_TRC("#" << getFD() << " got Expect: 100-continue, sending Continue"); + // FIXME: should validate authentication headers early too. + send("HTTP/1.1 100 Continue\r\n\r\n", + sizeof("HTTP/1.1 100 Continue\r\n\r\n") - 1); + _sentHTTPContinue = true; + } } catch (const Poco::Exception& exc) { diff --git a/net/Socket.hpp b/net/Socket.hpp index f64b2c362..7d50495ce 100644 --- a/net/Socket.hpp +++ b/net/Socket.hpp @@ -801,6 +801,7 @@ public: _bytesRecvd(0), _wsState(WSState::HTTP), _closed(false), + _sentHTTPContinue(false), _shutdownSignalled(false) { LOG_DBG("StreamSocket ctor #" << fd); @@ -1154,6 +1155,9 @@ protected: /// True if we are already closed. bool _closed; + /// True if we've received a Continue in response to an Expect: 100-continue + bool _sentHTTPContinue; + /// True when shutdown was requested via shutdown(). bool _shutdownSignalled; }; diff --git a/test/Makefile.am b/test/Makefile.am index 2c8a22785..ab0cfcdb9 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -17,7 +17,7 @@ noinst_LTLIBRARIES = \ unit-timeout.la unit-prefork.la \ unit-storage.la unit-client.la \ unit-admin.la unit-tilecache.la \ - unit-fuzz.la unit-oob.la unit-oauth.la \ + unit-fuzz.la unit-oob.la unit-http.la unit-oauth.la \ unit-wopi.la unit-wopi-saveas.la \ unit-wopi-ownertermination.la unit-wopi-versionrestore.la \ unit-wopi-documentconflict.la unit_wopi_renamefile.la @@ -86,6 +86,7 @@ fakesockettest_LDADD = $(CPPUNIT_LIBS) # unit test modules: unit_oob_la_SOURCES = UnitOOB.cpp +unit_http_la_SOURCES = UnitHTTP.cpp unit_fuzz_la_SOURCES = UnitFuzz.cpp unit_admin_la_SOURCES = UnitAdmin.cpp unit_admin_la_LIBADD = $(CPPUNIT_LIBS) @@ -127,7 +128,8 @@ check-local: TESTS = unit-convert.la unit-prefork.la unit-tilecache.la unit-timeout.la \ unit-oauth.la unit-wopi.la unit-wopi-saveas.la \ unit-wopi-ownertermination.la unit-wopi-versionrestore.la \ - unit-wopi-documentconflict.la unit_wopi_renamefile.la + unit-wopi-documentconflict.la unit_wopi_renamefile.la \ + unit-http.la # TESTS = unit-client.la # TESTS += unit-admin.la # TESTS += unit-storage.la diff --git a/test/UnitHTTP.cpp b/test/UnitHTTP.cpp new file mode 100644 index 000000000..d0530fe10 --- /dev/null +++ b/test/UnitHTTP.cpp @@ -0,0 +1,97 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <config.h> + +#include <cassert> + +#include <helpers.hpp> +#include <Poco/Util/Application.h> +#include <Poco/Net/StringPartSource.h> +#include <Poco/Net/HTMLForm.h> +#include <Poco/Net/HTTPRequest.h> +#include <Poco/Net/HTTPResponse.h> +#include <Poco/Net/HTTPSClientSession.h> + +#include <Log.hpp> +#include <Util.hpp> +#include <Unit.hpp> + +class UnitHTTP : public UnitWSD +{ +public: + UnitHTTP() + { + } + + void configure(Poco::Util::LayeredConfiguration& config) override + { + UnitWSD::configure(config); + // force HTTPS - to test harder + config.setBool("ssl.enable", true); + } + + // FIXME: can hook with (UnitWSD::get().handleHttpRequest(request, message, socket)) ... + void invokeTest() override + { + for (int i = 0; i < 3; ++i) + { + std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(Poco::URI(helpers::getTestServerURI()))); + + std::string sent = "Hello world test\n"; + + Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to/txt"); + + switch(i) + { + case 0: + request.setExpectContinue(false); + break; + case 1: + request.setExpectContinue(true); + break; + default: + break; + } + Poco::Net::HTMLForm form; + form.setEncoding(Poco::Net::HTMLForm::ENCODING_MULTIPART); + form.set("format", "txt"); + form.addPart("data", new Poco::Net::StringPartSource(sent, "text/plain", "foobaa.txt")); + form.prepareSubmit(request); + form.write(session->sendRequest(request)); + + Poco::Net::HTTPResponse response; + std::stringstream actualStream; + std::istream& responseStream = session->receiveResponse(response); + Poco::StreamCopier::copyStream(responseStream, actualStream); + + std::string responseStr = actualStream.str(); + responseStr.erase(0,3); // remove utf-8 bom. + + if (sent != responseStr) + { + std::cerr << "Test " << i << " failed - mismatching string '" << responseStr << " vs. '" << sent << "'\n"; + exitTest(TestResult::Failed); + return; + } + } + // Give those convertors time to save and cleanup. + std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + + std::cerr << "All tests passed.\n"; + exitTest(TestResult::Ok); + } +}; + +UnitBase *unit_create_wsd(void) +{ + return new UnitHTTP(); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ commit 134cbba2cb5620a1bd298ccb5e0233f8c74f935c Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Wed May 22 02:38:39 2019 +0100 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Thu May 23 12:37:02 2019 +0100 Avoid exceptions in some shutdown corner cases. Change-Id: I1c301dc96d925fd5d74c00bf4b9417782822a997 diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 98f94cc12..5ecaa3b0c 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -1957,11 +1957,14 @@ void ConvertToBroker::removeFile(const std::string &uriOrig) { if (!uriOrig.empty()) { - // Remove source file and directory - Poco::Path path = uriOrig; - Poco::File(path).remove(); - Poco::File(path.makeParent()).remove(); - FileUtil::removeFile(uriOrig); + try { + // Remove source file and directory + Poco::Path path = uriOrig; + Poco::File(path).remove(); + Poco::File(path.makeParent()).remove(); + } catch (const std::exception &ex) { + LOG_ERR("Error while removing conversion temporary: '" << uriOrig << "' - " << ex.what()); + } } } commit 3a2dbfa23e5c8916b6a39226d3dd78f8b06e0eb1 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Tue May 21 19:50:17 2019 +0100 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Thu May 23 12:37:02 2019 +0100 tdf#123482 - cleanup convert-to folder even more reliably. Problems could occur if exceptiosn thrown when parsing the input stream. Change-Id: Id82b3816450194164fc2093554c730b4a94acef1 Reviewed-on: https://gerrit.libreoffice.org/72695 Reviewed-by: Michael Meeks <michael.me...@collabora.com> Tested-by: Michael Meeks <michael.me...@collabora.com> diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 72cedf852..98f94cc12 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -1950,13 +1950,18 @@ ConvertToBroker::ConvertToBroker(const std::string& uri, ConvertToBroker::~ConvertToBroker() { NumConverters--; - if (!_uriOrig.empty()) + removeFile(_uriOrig); +} + +void ConvertToBroker::removeFile(const std::string &uriOrig) +{ + if (!uriOrig.empty()) { // Remove source file and directory - Poco::Path path = _uriOrig; + Poco::Path path = uriOrig; Poco::File(path).remove(); Poco::File(path.makeParent()).remove(); - FileUtil::removeFile(_uriOrig); + FileUtil::removeFile(uriOrig); } } diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index e470d0c8c..4eb89f751 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -484,6 +484,9 @@ public: /// How many live conversions are running. static size_t getInstanceCount(); + + /// Cleanup path and its parent + static void removeFile(const std::string &uri); }; #endif diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 9bdd7d434..ce16684f8 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -573,6 +573,9 @@ class ConvertToPartHandler : public PartHandler public: std::string getFilename() const { return _filename; } + /// Afterwards someone else is responsible for cleaning that up. + void takeFile() { _filename.clear(); } + ConvertToPartHandler(bool convertTo = false) : _convertTo(convertTo) { @@ -580,6 +583,11 @@ public: virtual ~ConvertToPartHandler() { + if (!_filename.empty()) + { + LOG_TRC("Remove un-handled temporary file '" << _filename << "'"); + ConvertToBroker::removeFile(_filename); + } } virtual void handlePart(const MessageHeader& header, std::istream& stream) override @@ -2387,6 +2395,7 @@ private: LOG_DBG("New DocumentBroker for docKey [" << docKey << "]."); auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey); + handler.takeFile(); cleanupDocBrokers(); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits