[Libreoffice-commits] online.git: loolwsd/IoUtil.cpp loolwsd/LOOLWebSocket.hpp loolwsd/test
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Fri Nov 25 03:48:50 UTC 2016
loolwsd/IoUtil.cpp | 42 +++++++++++-------------------------------
loolwsd/LOOLWebSocket.hpp | 46 +++++++++++++++++++++++++++++++++-------------
loolwsd/test/helpers.hpp | 28 +++++-----------------------
3 files changed, 49 insertions(+), 67 deletions(-)
New commits:
commit 84607b43a31574533471defcb4756ba855f835f1
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Thu Nov 24 18:26:07 2016 -0500
loolwsd: support reading long messages directly
Since Poco receiveFrame expected the buffer to be
at least as large as the frame, otherwise the socket
had to be closed, we sent a 'nextmessage' frame
before sending large frames with the payload size.
This caused many problems, not least related to threading
and lack of atomicity when sending large frames.
There is another API in Poco that doesn't have this
strict requirement, one that expects Poco::Buffer
and resizes it as necessary.
One potential issue is the possibility that a malicious
attacker might send very large frames to force the
server into allocating large buffers to read them,
thereby destibilizing the server, if not bringing it
down altogether.
Change-Id: I05014d54c3a1464f629ed82d761a7a65e4941985
Reviewed-on: https://gerrit.libreoffice.org/31184
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/IoUtil.cpp b/loolwsd/IoUtil.cpp
index 035dcf8..adf5c55 100644
--- a/loolwsd/IoUtil.cpp
+++ b/loolwsd/IoUtil.cpp
@@ -50,17 +50,17 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
// Timeout given is in microseconds.
static const Poco::Timespan waitTime(POLL_TIMEOUT_MS * 1000);
- const auto bufferSize = READ_BUFFER_SIZE * 100;
+ constexpr auto bufferSize = READ_BUFFER_SIZE * 8;
+
int flags = 0;
int n = -1;
bool stop = false;
std::vector<char> payload(bufferSize);
+ Poco::Buffer<char> buffer(bufferSize);
try
{
ws->setReceiveTimeout(0);
- payload.resize(0);
-
for (;;)
{
stop = stopPredicate();
@@ -79,10 +79,12 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
try
{
- payload.resize(payload.capacity());
+ payload.resize(0);
+ buffer.resize(0);
n = -1;
- n = ws->receiveFrame(payload.data(), payload.capacity(), flags);
- payload.resize(n > 0 ? n : 0);
+ n = ws->receiveFrame(buffer, flags);
+ LOG_WRN("GOT: [" << LOOLProtocol::getAbbreviatedMessage(buffer.begin(), buffer.size()) << "]");
+ payload.insert(payload.end(), buffer.begin(), buffer.end());
}
catch (const Poco::TimeoutException&)
{
@@ -99,7 +101,7 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
assert(n > 0);
- const std::string firstLine = LOOLProtocol::getFirstLine(payload);
+ const std::string firstLine = LOOLProtocol::getFirstLine(buffer.begin(), buffer.size());
if ((flags & WebSocket::FrameFlags::FRAME_FLAG_FIN) != WebSocket::FrameFlags::FRAME_FLAG_FIN)
{
// One WS message split into multiple frames.
@@ -107,8 +109,7 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
LOG_WRN("SocketProcessor [" << name << "]: Receiving multi-parm frame.");
while (true)
{
- char buffer[READ_BUFFER_SIZE * 10];
- n = ws->receiveFrame(buffer, sizeof(buffer), flags);
+ n = ws->receiveFrame(buffer, flags);
if (n <= 0 || (flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_CLOSE)
{
LOG_WRN("SocketProcessor [" << name << "]: Connection closed while reading multiframe message.");
@@ -116,7 +117,7 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
break;
}
- payload.insert(payload.end(), buffer, buffer + n);
+ payload.insert(payload.end(), buffer.begin(), buffer.end());
if ((flags & WebSocket::FrameFlags::FRAME_FLAG_FIN) == WebSocket::FrameFlags::FRAME_FLAG_FIN)
{
// No more frames.
@@ -124,27 +125,6 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
}
}
}
- else
- {
- int size = 0;
- Poco::StringTokenizer tokens(firstLine, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
- // Check if it is a "nextmessage:" and in that case read the large
- // follow-up message separately, and handle that only.
- if (tokens.count() == 2 && tokens[0] == "nextmessage:" &&
- LOOLProtocol::getTokenInteger(tokens[1], "size", size) && size > 0)
- {
- LOG_TRC("SocketProcessor [" << name << "]: Getting large message of " << size << " bytes.");
- if (size > MAX_MESSAGE_SIZE)
- {
- LOG_ERR("SocketProcessor [" << name << "]: Large-message size (" << size << ") over limit or invalid.");
- }
- else
- {
- payload.resize(size);
- continue;
- }
- }
- }
if (n <= 0 || (flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_CLOSE)
{
diff --git a/loolwsd/LOOLWebSocket.hpp b/loolwsd/LOOLWebSocket.hpp
index e10af2a..fb1d88b 100644
--- a/loolwsd/LOOLWebSocket.hpp
+++ b/loolwsd/LOOLWebSocket.hpp
@@ -108,6 +108,39 @@ public:
return -1;
}
+
+ /// Wrapper for Poco::Net::WebSocket::receiveFrame() that handles PING frames
+ /// (by replying with a PONG frame) and PONG frames. PONG frames are ignored.
+ /// Should we also factor out the handling of non-final and continuation frames into this?
+ int receiveFrame(Poco::Buffer<char>& buffer, int& flags)
+ {
+#ifdef ENABLE_DEBUG
+ // Delay receiving the frame
+ std::this_thread::sleep_for(getWebSocketDelay());
+#endif
+ // Timeout given is in microseconds.
+ static const Poco::Timespan waitTime(POLL_TIMEOUT_MS * 1000);
+
+ while (poll(waitTime, Poco::Net::Socket::SELECT_READ))
+ {
+ const int n = Poco::Net::WebSocket::receiveFrame(buffer, flags);
+ if (n > 0 && (flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_PING)
+ {
+ sendFrame(buffer.begin(), n, WebSocket::FRAME_FLAG_FIN | WebSocket::FRAME_OP_PONG);
+ }
+ else if ((flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_PONG)
+ {
+ // In case we do send pongs in the future.
+ }
+ else
+ {
+ return n;
+ }
+ }
+
+ return -1;
+ }
+
/// Wrapper for Poco::Net::WebSocket::sendFrame() that handles large frames.
int sendFrame(const char* buffer, const int length, const int flags = FRAME_TEXT)
{
@@ -117,19 +150,6 @@ public:
#endif
std::unique_lock<std::mutex> lock(_mutex);
- // Size after which messages will be sent preceded with
- // 'nextmessage' frame to let the receiver know in advance
- // the size of larger coming message. All messages up to this
- // size are considered small messages.
- constexpr int SMALL_MESSAGE_SIZE = READ_BUFFER_SIZE / 2;
-
- if (length > SMALL_MESSAGE_SIZE)
- {
- const std::string nextmessage = "nextmessage: size=" + std::to_string(length);
- Poco::Net::WebSocket::sendFrame(nextmessage.data(), nextmessage.size());
- Log::debug("Message is long, sent " + nextmessage);
- }
-
const int result = Poco::Net::WebSocket::sendFrame(buffer, length, flags);
lock.unlock();
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index 30aeac1..7e5f039 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -197,6 +197,7 @@ std::vector<char> getResponseMessage(LOOLWebSocket& ws, const std::string& prefi
int retries = timeoutMs / 500;
const Poco::Timespan waitTime(retries ? timeoutMs * 1000 / retries : timeoutMs * 1000);
std::vector<char> response;
+ Poco::Buffer<char> buffer(READ_BUFFER_SIZE);
bool timedout = false;
ws.setReceiveTimeout(0);
@@ -210,9 +211,10 @@ std::vector<char> getResponseMessage(LOOLWebSocket& ws, const std::string& prefi
timedout = false;
}
- response.resize(READ_BUFFER_SIZE);
- int bytes = ws.receiveFrame(response.data(), response.size(), flags);
- response.resize(bytes >= 0 ? bytes : 0);
+ response.resize(0);
+ buffer.resize(0);
+ const int bytes = ws.receiveFrame(buffer, flags);
+ response.insert(response.end(), buffer.begin(), buffer.end());
std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl;
const auto message = LOOLProtocol::getFirstLine(response);
if (bytes > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE)
@@ -221,26 +223,6 @@ std::vector<char> getResponseMessage(LOOLWebSocket& ws, const std::string& prefi
{
return response;
}
- else if (LOOLProtocol::matchPrefix("nextmessage", message))
- {
- int size = 0;
- if (LOOLProtocol::getTokenIntegerFromMessage(message, "size", size) && size > 0)
- {
- response.resize(size);
- bytes = ws.receiveFrame(response.data(), response.size(), flags);
- response.resize(bytes >= 0 ? bytes : 0);
- std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl;
- if (bytes > 0 &&
- LOOLProtocol::matchPrefix(prefix, LOOLProtocol::getFirstLine(response)))
- {
- return response;
- }
- }
- }
- }
- else
- {
- response.resize(0);
}
if (bytes <= 0)
More information about the Libreoffice-commits
mailing list