[Libreoffice-commits] online.git: common/Common.hpp wsd/LOOLWSD.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Mon Apr 24 04:49:52 UTC 2017
common/Common.hpp | 4
wsd/LOOLWSD.cpp | 238 +++++++++++++++++++++++++++++-------------------------
2 files changed, 134 insertions(+), 108 deletions(-)
New commits:
commit c851c3e93be565dfa14906febbdf6208e9d422cd
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Apr 24 00:44:14 2017 -0400
wsd: correctly remove request from socket buffer
POST requests require the full request to be
left in the socket buffer to be parsed in full.
But GET requests, especially WS upgrade, must
have the request cleared from the socket, as
there is more data expected to be read after
the upgrade, which happens by the DocBroker
thread, so clearing the buffer must be done
before the upgrade.
This patch accomodates these two conflicting
cases and refactors the code slightly to
make it more structured and readable.
Change-Id: Ia7357a745a3900f986099ba14af2a0946023018b
Reviewed-on: https://gerrit.libreoffice.org/36873
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/common/Common.hpp b/common/Common.hpp
index 09bafad3..b0b018b2 100644
--- a/common/Common.hpp
+++ b/common/Common.hpp
@@ -34,8 +34,8 @@ constexpr auto CHILD_URI = "/loolws/child?";
constexpr auto NEW_CHILD_URI = "/loolws/newchild?";
constexpr auto LO_JAIL_SUBPATH = "lo";
-/// The HTTP response User-Agent. TODO: Include version.
-constexpr auto HTTP_AGENT_STRING = "LOOLWSD Agent";
+/// The HTTP response User-Agent.
+constexpr auto HTTP_AGENT_STRING = "LOOLWSD Agent " LOOLWSD_VERSION;
// The client port number, both loolwsd and the kits have this.
extern int ClientPortNumber;
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 15391c4c..c604206e 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1598,46 +1598,47 @@ private:
Poco::URI requestUri(request.getURI());
std::vector<std::string> reqPathSegs;
requestUri.getPathSegments(reqPathSegs);
+ const StringTokenizer reqPathTokens(request.getURI(), "/?", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
- // File server
- if (reqPathSegs.size() >= 1 && reqPathSegs[0] == "loleaflet")
+ if (request.getMethod() == HTTPRequest::HTTP_GET ||
+ request.getMethod() == HTTPRequest::HTTP_HEAD)
{
- handleFileServerRequest(request, message);
- }
- // Admin connections
- else if (reqPathSegs.size() >= 2 && reqPathSegs[0] == "lool" && reqPathSegs[1] == "adminws")
- {
- LOG_ERR("Admin request: " << request.getURI());
- if (AdminSocketHandler::handleInitialRequest(_socket, request))
- {
- // Hand the socket over to the Admin poll.
- Admin::instance().insertNewSocket(socket);
- socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
- }
- }
- // Client post and websocket connections
- else if ((request.getMethod() == HTTPRequest::HTTP_GET ||
- request.getMethod() == HTTPRequest::HTTP_HEAD) &&
- request.getURI() == "/")
- {
- handleRootRequest(request);
- }
- else if (request.getMethod() == HTTPRequest::HTTP_GET && request.getURI() == "/favicon.ico")
- {
- handleFaviconRequest(request);
- }
- else if (request.getMethod() == HTTPRequest::HTTP_GET && request.getURI() == "/hosting/discovery")
- {
- handleWopiDiscoveryRequest(request);
- }
- else
- {
- StringTokenizer reqPathTokens(request.getURI(), "/?", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
+ // Clear the request header already consumed.
+ in.erase(in.begin(), itBody);
+
+ // File server
if (!(request.find("Upgrade") != request.end() && Poco::icompare(request["Upgrade"], "websocket") == 0) &&
- reqPathTokens.count() > 0 && reqPathTokens[0] == "lool")
+ reqPathTokens.count() >= 5 && reqPathTokens[0] == "lool")
{
// All post requests have url prefix 'lool'.
- socketOwnership = handlePostRequest(request, message);
+ handleFileDownloadRequest(request);
+ }
+ else if (reqPathSegs.size() >= 1 && reqPathSegs[0] == "loleaflet")
+ {
+ handleFileServerRequest(request, message);
+ }
+ else if (reqPathSegs.size() >= 2 && reqPathSegs[0] == "lool" && reqPathSegs[1] == "adminws")
+ {
+ // Admin connections
+ LOG_INF("Admin request: " << request.getURI());
+ if (AdminSocketHandler::handleInitialRequest(_socket, request))
+ {
+ // Hand the socket over to the Admin poll.
+ Admin::instance().insertNewSocket(socket);
+ socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
+ }
+ }
+ else if (request.getURI() == "/")
+ {
+ handleRootRequest(request);
+ }
+ else if (request.getURI() == "/favicon.ico")
+ {
+ handleFaviconRequest(request);
+ }
+ else if (request.getURI() == "/hosting/discovery")
+ {
+ handleWopiDiscoveryRequest(request);
}
else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws" &&
request.find("Upgrade") != request.end() && Poco::icompare(request["Upgrade"], "websocket") == 0)
@@ -1646,19 +1647,31 @@ private:
}
else
{
- LOG_ERR("Unknown resource: " << request.getURI());
-
- // Bad request.
- std::ostringstream oss;
- oss << "HTTP/1.1 400\r\n"
- << "Date: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n"
- << "User-Agent: LOOLWSD WOPI Agent\r\n"
- << "Content-Length: 0\r\n"
- << "\r\n";
- socket->send(oss.str());
- socket->shutdown();
+ throw std::runtime_error("Bad request.");
+ }
+
+ return socketOwnership;
+ }
+ else if (request.getMethod() == HTTPRequest::HTTP_POST)
+ {
+ if (reqPathSegs.size() >= 1 && reqPathSegs[0] == "loleaflet")
+ {
+ handleFileServerRequest(request, message);
+ return socketOwnership;
+ }
+
+ if (!(request.find("Upgrade") != request.end() && Poco::icompare(request["Upgrade"], "websocket") == 0) &&
+ reqPathTokens.count() > 0 && reqPathTokens[0] == "lool")
+ {
+ // All post requests have url prefix 'lool'.
+ socketOwnership = handlePostRequest(request, message);
+ in.erase(in.begin(), itBody);
+ return socketOwnership;
}
}
+
+ LOG_ERR("#" << socket->getFD() << " Unknown request: [" <<
+ LOOLProtocol::getAbbreviatedMessage(in) << "].");
}
catch (const std::exception& exc)
{
@@ -1668,10 +1681,17 @@ private:
LOOLProtocol::getAbbreviatedMessage(in) << "]: " << exc.what());
}
- // if we succeeded - remove the request from our input buffer
- // we expect one request per socket
- in.erase(in.begin(), itBody);
- return socketOwnership;
+ // Bad request.
+ std::ostringstream oss;
+ oss << "HTTP/1.1 400\r\n"
+ << "Date: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n"
+ << "User-Agent: " << HTTP_AGENT_STRING << "\r\n"
+ << "Content-Length: 0\r\n"
+ << "\r\n";
+ socket->send(oss.str());
+ socket->shutdown();
+
+ return SocketHandlerInterface::SocketOwnership::UNCHANGED;
}
int getPollEvents(std::chrono::steady_clock::time_point /* now */,
@@ -1946,75 +1966,81 @@ private:
}
}
}
- else if (tokens.count() >= 6)
- {
- LOG_INF("File download request.");
- // TODO: Check that the user in question has access to this file!
- // 1. Validate the dockey
- std::string decodedUri;
- URI::decode(tokens[2], decodedUri);
- const auto docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
- std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
- auto docBrokerIt = DocBrokers.find(docKey);
- if (docBrokerIt == DocBrokers.end())
- {
- throw BadRequestException("DocKey [" + docKey + "] is invalid.");
- }
+ throw BadRequestException("Invalid or unknown request.");
+ }
- // 2. Cross-check if received child id is correct
- if (docBrokerIt->second->getJailId() != tokens[3])
- {
- throw BadRequestException("ChildId does not correspond to docKey");
- }
+ void handleFileDownloadRequest(const Poco::Net::HTTPRequest& request)
+ {
+ LOG_INF("File download request.");
+
+ Poco::Net::HTTPResponse response;
+ auto socket = _socket.lock();
+
+ StringTokenizer tokens(request.getURI(), "/?");
+ // TODO: Check that the user in question has access to this file!
+
+ // 1. Validate the dockey
+ std::string decodedUri;
+ URI::decode(tokens[2], decodedUri);
+ const auto docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
+ std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
+ auto docBrokerIt = DocBrokers.find(docKey);
+ if (docBrokerIt == DocBrokers.end())
+ {
+ throw BadRequestException("DocKey [" + docKey + "] is invalid.");
+ }
+
+ // 2. Cross-check if received child id is correct
+ if (docBrokerIt->second->getJailId() != tokens[3])
+ {
+ throw BadRequestException("ChildId does not correspond to docKey");
+ }
- // 3. Don't let user download the file in main doc directory containing
- // the document being edited otherwise we will end up deleting main directory
- // after download finishes
- if (docBrokerIt->second->getJailId() == tokens[4])
+ // 3. Don't let user download the file in main doc directory containing
+ // the document being edited otherwise we will end up deleting main directory
+ // after download finishes
+ if (docBrokerIt->second->getJailId() == tokens[4])
+ {
+ throw BadRequestException("RandomDir cannot be equal to ChildId");
+ }
+ docBrokersLock.unlock();
+
+ std::string fileName;
+ bool responded = false;
+ URI::decode(tokens[5], fileName);
+ const Path filePath(LOOLWSD::ChildRoot + tokens[3]
+ + JAILED_DOCUMENT_ROOT + tokens[4] + "/" + fileName);
+ LOG_INF("HTTP request for: " << filePath.toString());
+ if (filePath.isAbsolute() && File(filePath).exists())
+ {
+ std::string contentType = getContentType(fileName);
+ if (Poco::Path(fileName).getExtension() == "pdf")
{
- throw BadRequestException("RandomDir cannot be equal to ChildId");
+ contentType = "application/pdf";
+ response.set("Content-Disposition", "attachment; filename=\"" + fileName + "\"");
}
- docBrokersLock.unlock();
- std::string fileName;
- bool responded = false;
- URI::decode(tokens[5], fileName);
- const Path filePath(LOOLWSD::ChildRoot + tokens[3]
- + JAILED_DOCUMENT_ROOT + tokens[4] + "/" + fileName);
- LOG_INF("HTTP request for: " << filePath.toString());
- if (filePath.isAbsolute() && File(filePath).exists())
+ try
{
- std::string contentType = getContentType(fileName);
- if (Poco::Path(fileName).getExtension() == "pdf")
- {
- contentType = "application/pdf";
- response.set("Content-Disposition", "attachment; filename=\"" + fileName + "\"");
- }
-
- try
- {
- HttpHelper::sendFile(socket, filePath.toString(), contentType, response);
- responded = true;
- }
- catch (const Exception& exc)
- {
- LOG_ERR("Error sending file to client: " << exc.displayText() <<
- (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
- }
-
- FileUtil::removeFile(File(filePath.parent()).path(), true);
+ HttpHelper::sendFile(socket, filePath.toString(), contentType, response);
+ responded = true;
}
- else
+ catch (const Exception& exc)
{
- LOG_ERR("Download file [" << filePath.toString() << "] not found.");
+ LOG_ERR("Error sending file to client: " << exc.displayText() <<
+ (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
}
- (void)responded;
- return socketOwnership;
+ FileUtil::removeFile(File(filePath.parent()).path(), true);
+ }
+ else
+ {
+ LOG_ERR("Download file [" << filePath.toString() << "] not found.");
}
- throw BadRequestException("Invalid or unknown request.");
+ // TODO: Use this to send failure response.
+ (void)responded;
}
SocketHandlerInterface::SocketOwnership handleClientWsUpgrade(const Poco::Net::HTTPRequest& request, const std::string& url)
More information about the Libreoffice-commits
mailing list