[Libreoffice-commits] online.git: Branch 'private/Ashod/nonblocking' - 5 commits - common/Session.cpp net/ServerSocket.hpp net/Socket.hpp test/httpwstest.cpp wsd/FileServer.cpp wsd/FileServer.hpp wsd/LOOLWSD.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Tue Feb 28 05:31:48 UTC 2017
common/Session.cpp | 6 +--
net/ServerSocket.hpp | 2 -
net/Socket.hpp | 80 +++++++++++++++++++++++++++------------------------
test/httpwstest.cpp | 9 +----
wsd/FileServer.cpp | 15 +++++----
wsd/FileServer.hpp | 5 +--
wsd/LOOLWSD.cpp | 74 +++++++++++++++++++++++++++++++++++------------
7 files changed, 116 insertions(+), 75 deletions(-)
New commits:
commit 0c71a59fc88770dc7009bd2287bc1d4c3aa4e54f
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Feb 27 23:36:14 2017 -0500
nb: handle bad requests with 400 response
Change-Id: I15ecc06a933a88f5ccdd0acfd16aac8b357f29df
diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 3563c24..f02eac5 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -226,15 +226,12 @@ void HTTPWSTest::testBadRequest()
{
try
{
- // Try to load a fake document and get its status.
- const std::string documentURL = "/lool/file%3A%2F%2F%2Ffake.doc/ws";
+ // Try to load a bogus url.
+ const std::string documentURL = "/lol/file%3A%2F%2F%2Ffake.doc";
Poco::Net::HTTPResponse response;
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, documentURL);
std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri));
- // This should result in Bad Request, but results in:
- // WebSocket Exception: Missing Sec-WebSocket-Key in handshake request
- // So Service Unavailable is returned.
request.set("Connection", "Upgrade");
request.set("Upgrade", "websocket");
@@ -244,7 +241,7 @@ void HTTPWSTest::testBadRequest()
session->setKeepAlive(true);
session->sendRequest(request);
session->receiveResponse(response);
- CPPUNIT_ASSERT_EQUAL(Poco::Net::HTTPResponse::HTTPResponse::HTTP_SERVICE_UNAVAILABLE, response.getStatus());
+ CPPUNIT_ASSERT_EQUAL(Poco::Net::HTTPResponse::HTTPResponse::HTTP_BAD_REQUEST, response.getStatus());
}
catch (const Poco::Exception& exc)
{
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index cedfd87..8315987 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2765,7 +2765,16 @@ private:
else
{
LOG_ERR("Unknown resource: " << request.getURI());
- // response.setStatusAndReason(HTTPResponse::HTTP_BAD_REQUEST);
+
+ // 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();
}
}
}
commit 4fa5bc814ae17abfbad725c59cc215a593d29b91
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Feb 27 22:38:18 2017 -0500
nb: proper POST body processing
Change-Id: Ic37094e50979e14d2862ae32088295b42d9c4931
diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index 8462386..c4f83f5 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -108,7 +108,7 @@ bool FileServerRequestHandler::isAdminLoggedIn(HTTPServerRequest& request, HTTPS
return false;
}
-void FileServerRequestHandler::handleRequest(const HTTPRequest& request, const std::shared_ptr<StreamSocket>& socket)
+void FileServerRequestHandler::handleRequest(const HTTPRequest& request, Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& socket)
{
try
{
@@ -128,7 +128,7 @@ void FileServerRequestHandler::handleRequest(const HTTPRequest& request, const s
const std::string endPoint = requestSegments[requestSegments.size() - 1];
if (endPoint == loleafletHtml)
{
- preprocessFile(request, socket);
+ preprocessFile(request, message, socket);
return;
}
@@ -229,7 +229,7 @@ std::string FileServerRequestHandler::getRequestPathname(const HTTPRequest& requ
return path;
}
-void FileServerRequestHandler::preprocessFile(const HTTPRequest& request, const std::shared_ptr<StreamSocket>& socket)
+void FileServerRequestHandler::preprocessFile(const HTTPRequest& request, Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& socket)
{
const auto host = ((LOOLWSD::isSSLEnabled() || LOOLWSD::isSSLTermination()) ? "wss://" : "ws://") + (LOOLWSD::ServerName.empty() ? request.getHost() : LOOLWSD::ServerName);
const auto path = Poco::Path(LOOLWSD::FileServerRoot, getRequestPathname(request));
@@ -255,9 +255,10 @@ void FileServerRequestHandler::preprocessFile(const HTTPRequest& request, const
StreamCopier::copyToString(file, preprocess);
file.close();
- //TODO: FIXME: Is this the correct way to get these values? Used to be done using HTTPForm.
- const std::string& accessToken = request.get("access_token", "");
- const std::string& accessTokenTtl = request.get("access_token_ttl", "");
+ HTMLForm form(request, message);
+ const std::string& accessToken = form.get("access_token", "");
+ const std::string& accessTokenTtl = form.get("access_token_ttl", "");
+ LOG_TRC("access_token=" << accessToken << ", access_token_ttl=" << accessTokenTtl);
// Escape bad characters in access token.
// This is placed directly in javascript in loleaflet.html, we need to make sure
diff --git a/wsd/FileServer.hpp b/wsd/FileServer.hpp
index 7738968..a1ad497 100644
--- a/wsd/FileServer.hpp
+++ b/wsd/FileServer.hpp
@@ -14,6 +14,7 @@
#include <string>
+#include <Poco/MemoryStream.h>
#include <Poco/Net/HTTPRequestHandler.h>
#include <Poco/Net/HTTPServerRequest.h>
#include <Poco/Net/HTTPServerResponse.h>
@@ -25,13 +26,13 @@ class FileServerRequestHandler
{
static std::string getRequestPathname(const Poco::Net::HTTPRequest& request);
- static void preprocessFile(const Poco::Net::HTTPRequest& request, const std::shared_ptr<StreamSocket>& socket);
+ static void preprocessFile(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& socket);
public:
/// Evaluate if the cookie exists, and if not, ask for the credentials.
static bool isAdminLoggedIn(Poco::Net::HTTPServerRequest& request, Poco::Net::HTTPServerResponse& response);
- static void handleRequest(const Poco::Net::HTTPRequest& request, const std::shared_ptr<StreamSocket>& socket);
+ static void handleRequest(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& socket);
};
#endif
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 19ef4e0..cedfd87 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2663,7 +2663,22 @@ private:
}
auto socket = _socket.lock();
- Poco::MemoryInputStream message(&socket->_inBuffer[0], socket->_inBuffer.size());
+ std::vector<char>& in = socket->_inBuffer;
+
+ // Find the end of the header, if any.
+ static const std::string marker("\r\n\r\n");
+ auto itBody = std::search(in.begin(), in.end(),
+ marker.begin(), marker.end());
+ if (itBody == in.end())
+ {
+ LOG_TRC("#" << socket->getFD() << " doesn't have enough data yet.");
+ return;
+ }
+
+ // Skip the marker.
+ itBody += marker.size();
+
+ Poco::MemoryInputStream message(&in[0], in.size());
Poco::Net::HTTPRequest request;
try
{
@@ -2682,13 +2697,19 @@ private:
logger << Log::end;
- // if we succeeded - remove that from our input buffer
- // FIXME: We should check if this is GET or POST. For GET, we only
- // can have a single request (headers only). For POST, we can/should
- // use Poco HTMLForm to parse the post message properly.
- // Otherwise, we should catch exceptions from the previous read/parse
- // and assume we don't have sufficient data, so we wait some more.
- socket->_inBuffer.clear();
+ const std::streamsize contentLength = request.getContentLength();
+ const auto offset = itBody - in.begin();
+ const std::streamsize available = in.size() - offset;
+
+ if (contentLength != Poco::Net::HTTPMessage::UNKNOWN_CONTENT_LENGTH && available < contentLength)
+ {
+ LOG_DBG("Not enough content yet: ContentLength: " << contentLength << ", available: " << available);
+ return;
+ }
+
+ // if we succeeded - remove the request from our input buffer
+ // we expect one request per socket
+ in.clear();
}
catch (const std::exception& exc)
{
@@ -2706,7 +2727,7 @@ private:
// File server
if (reqPathSegs.size() >= 1 && reqPathSegs[0] == "loleaflet")
{
- handleFileServerRequest(request);
+ handleFileServerRequest(request, message);
}
// Admin LOOLWebSocket Connections
else if (reqPathSegs.size() >= 2 && reqPathSegs[0] == "lool" && reqPathSegs[1] == "adminws")
@@ -2735,7 +2756,7 @@ private:
reqPathTokens.count() > 0 && reqPathTokens[0] == "lool")
{
// All post requests have url prefix 'lool'.
- handlePostRequest(request);
+ handlePostRequest(request, message);
}
else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws")
{
@@ -2755,11 +2776,10 @@ private:
}
}
- void handleFileServerRequest(const Poco::Net::HTTPRequest& request)
+ void handleFileServerRequest(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
{
- LOG_DBG("FileServer request: " << request.getURI());
auto socket = _socket.lock();
- FileServerRequestHandler::handleRequest(request, socket);
+ FileServerRequestHandler::handleRequest(request, message, socket);
socket->shutdown();
}
@@ -2860,9 +2880,10 @@ private:
LOG_INF("Sent discovery.xml successfully.");
}
- void handlePostRequest(const Poco::Net::HTTPRequest& request)
+ void handlePostRequest(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
{
LOG_ERR("Post request: " << request.getURI());
+ (void)message;
// responded = handlePostRequest(request, response, id);
}
commit e6eea4bf8f5fbc4c79badf07221f8cc5e0bc2a77
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Feb 27 21:40:44 2017 -0500
nb: read the socket on accept
It's necessary to do the SSL handshake
and to get the request (if we get there)
on accepting so by the time we poll we know
what SSL needs to do next. No reason to
read on first poll when we should be
expecting a request upon connection anyway.
Change-Id: I8eecaad5f8450075d45e487702972418cad125bc
diff --git a/net/ServerSocket.hpp b/net/ServerSocket.hpp
index a6ad09e..d831172 100644
--- a/net/ServerSocket.hpp
+++ b/net/ServerSocket.hpp
@@ -85,7 +85,7 @@ public:
throw std::runtime_error(msg + std::strerror(errno) + ")");
}
- LOG_DBG( "Accepted client #" << clientSocket->getFD());
+ LOG_DBG("Accepted client #" << clientSocket->getFD());
_clientPoller.insertNewSocket(clientSocket);
}
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 1aa6ff9..fcbfe49 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -274,9 +274,12 @@ public:
/// Sockets are removed only when the handler return false.
void insertNewSocket(const std::shared_ptr<Socket>& newSocket)
{
- std::lock_guard<std::mutex> lock(_mutex);
- _newSockets.emplace_back(newSocket);
- wakeup();
+ if (newSocket)
+ {
+ std::lock_guard<std::mutex> lock(_mutex);
+ _newSockets.emplace_back(newSocket);
+ wakeup();
+ }
}
typedef std::function<void()> CallbackFn;
@@ -398,6 +401,35 @@ public:
send(str.data(), str.size(), flush);
}
+ /// Reads data by invoking readData() and buffering.
+ /// Return false iff the socket is closed.
+ virtual bool readIncomingData()
+ {
+ // SSL decodes blocks of 16Kb, so for efficiency we use the same.
+ char buf[16 * 1024];
+ ssize_t len;
+ do
+ {
+ // Drain the read buffer.
+ // TODO: Cap the buffer size, lest we grow beyond control.
+ do
+ {
+ len = readData(buf, sizeof(buf));
+ }
+ while (len < 0 && errno == EINTR);
+
+ if (len > 0)
+ {
+ assert (len <= ssize_t(sizeof(buf)));
+ _inBuffer.insert(_inBuffer.end(), &buf[0], &buf[len]);
+ }
+ // else poll will handle errors.
+ }
+ while (len == (sizeof(buf)));
+
+ return len != 0; // zero is eof / clean socket close.
+ }
+
/// Create a socket of type TSocket given an FD and a handler.
/// We need this helper since the handler needs a shared_ptr to the socket
/// but we can't have a shared_ptr in the ctor.
@@ -462,35 +494,6 @@ protected:
HandleResult::CONTINUE;
}
- /// Reads data by invoking readData() and buffering.
- /// Return false iff the socket is closed.
- virtual bool readIncomingData()
- {
- // SSL decodes blocks of 16Kb, so for efficiency we use the same.
- char buf[16 * 1024];
- ssize_t len;
- do
- {
- // Drain the read buffer.
- // TODO: Cap the buffer size, lest we grow beyond control.
- do
- {
- len = readData(buf, sizeof(buf));
- }
- while (len < 0 && errno == EINTR);
-
- if (len > 0)
- {
- assert (len <= ssize_t(sizeof(buf)));
- _inBuffer.insert(_inBuffer.end(), &buf[0], &buf[len]);
- }
- // else poll will handle errors.
- }
- while (len == (sizeof(buf)));
-
- return len != 0; // zero is eof / clean socket close.
- }
-
/// Override to write data out to socket.
virtual void writeOutgoingData()
{
@@ -582,7 +585,7 @@ namespace HttpHelper
<< "Content-Type: " << mediaType << "\r\n"
<< "\r\n";
- socket->send(oss.str(), false);
+ socket->send(oss.str());
std::ifstream file(path, std::ios::binary);
do
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index f0871d4..19ef4e0 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -3083,7 +3083,11 @@ class PlainSocketFactory : public SocketFactory
{
std::shared_ptr<Socket> create(const int fd) override
{
- return StreamSocket::create<StreamSocket>(fd, std::unique_ptr<SocketHandlerInterface>{ new ClientRequestDispatcher });
+ auto socket = StreamSocket::create<StreamSocket>(fd, std::unique_ptr<SocketHandlerInterface>{ new ClientRequestDispatcher });
+
+ // Read the request.
+ socket->readIncomingData();
+ return socket;
}
};
@@ -3092,7 +3096,11 @@ class SslSocketFactory : public SocketFactory
{
std::shared_ptr<Socket> create(const int fd) override
{
- return StreamSocket::create<SslStreamSocket>(fd, std::unique_ptr<SocketHandlerInterface>{ new ClientRequestDispatcher });
+ auto socket = StreamSocket::create<SslStreamSocket>(fd, std::unique_ptr<SocketHandlerInterface>{ new ClientRequestDispatcher });
+
+ // Do the ssl handshake and read the request.
+ socket->readIncomingData();
+ return socket;
}
};
#endif
commit d5e0e7bb781391f06d91bf8ef439a3fcccf2eb49
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Feb 27 21:35:06 2017 -0500
nb: logging
Change-Id: Ia67f746a6c71b4753d04b92472eddf1614c0d337
diff --git a/net/Socket.hpp b/net/Socket.hpp
index a5966aa..1aa6ff9 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -426,8 +426,8 @@ protected:
auto& log = Log::logger();
if (log.trace()) {
- LOG_TRC("Incoming data buffer " << _inBuffer.size() <<
- " closeSocket? " << _closed);
+ LOG_TRC("#" << getFD() << ": Incoming data buffer " << _inBuffer.size() <<
+ " bytes, closeSocket? " << closed);
log.dump("", &_inBuffer[0], _inBuffer.size());
}
@@ -507,12 +507,12 @@ protected:
if (log.trace()) {
if (len > 0)
{
- LOG_TRC("Wrote outgoing data " << len << " bytes");
+ LOG_TRC("#" << getFD() << ": Wrote outgoing data " << len << " bytes");
log.dump("", &_outBuffer[0], len);
}
else
{
- LOG_SYS("Wrote outgoing data " << len << " bytes");
+ LOG_SYS("#" << getFD() << ": Wrote outgoing data " << len << " bytes");
}
}
}
@@ -569,7 +569,10 @@ namespace HttpHelper
{
struct stat st;
if (stat(path.c_str(), &st) != 0)
+ {
+ LOG_WRN("Failed to stat [" << path << "]. File will not be sent.");
return;
+ }
std::ostringstream oss;
oss << "HTTP/1.1 200 OK\r\n"
diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index 01d2473..8462386 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -307,7 +307,7 @@ void FileServerRequestHandler::preprocessFile(const HTTPRequest& request, const
<< preprocess;
socket->send(oss.str());
- LOG_DBG("Sent file: " << path.toString());
+ LOG_DBG("Sent file: " << path.toString() << ": " << preprocess);
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 19ac013..f0871d4 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2791,7 +2791,7 @@ private:
auto socket = _socket.lock();
socket->send(oss.str());
socket->shutdown();
- LOG_INF("Sent discovery.xml successfully.");
+ LOG_INF("Sent / response successfully.");
}
void handleFaviconRequest(const Poco::Net::HTTPRequest& request)
commit 74a08dc88697992e3f1b5d3bd400313f778ff80e
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Feb 27 21:32:21 2017 -0500
nb: ignore empty payloads
When the socket is closed the last WS frame
will not have any payload, just a frame.
In this case the socket should still fire
handleMessage so this frame could trigger
application logic, however in this case
ClientSession has nothing to do, so we skip it.
Change-Id: Ia2b13026e31460ffceb8f9d9cfa39d36fbc57146
diff --git a/common/Session.cpp b/common/Session.cpp
index bea96ad..8258a2c 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -169,8 +169,8 @@ void Session::handleMessage(bool /*fin*/, WSOpCode /*code*/, std::vector<char> &
return;
}
- _handleInput(&data[0], data.size());
- return;
+ if (!data.empty())
+ _handleInput(&data[0], data.size());
}
catch (const Exception& exc)
{
@@ -184,8 +184,6 @@ void Session::handleMessage(bool /*fin*/, WSOpCode /*code*/, std::vector<char> &
LOG_ERR("Session::handleInput: Exception while handling [" <<
getAbbreviatedMessage(data) << "]: " << exc.what());
}
-
- return;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
More information about the Libreoffice-commits
mailing list