Skip to content

Commit e36800c

Browse files
obiltschnigobiltschnig
committed
- fixed SF# 594: Websocket fails with small masked payloads
1 parent 45fa903 commit e36800c

3 files changed

Lines changed: 119 additions & 87 deletions

File tree

‎Net/include/Poco/Net/WebSocketImpl.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//
22
// WebSocketImpl.h
33
//
4-
// $Id: //poco/1.4/Net/include/Poco/Net/WebSocketImpl.h#1 $
4+
// $Id: //poco/1.4/Net/include/Poco/Net/WebSocketImpl.h#5 $
55
//
66
// Library: Net
77
// Package: WebSocket
@@ -93,6 +93,7 @@ class Net_API WebSocketImpl: public StreamSocketImpl
9393
MAX_HEADER_LENGTH = 14
9494
};
9595

96+
int receiveNBytes(void* buffer, int bytes);
9697
virtual ~WebSocketImpl();
9798

9899
private:

‎Net/src/WebSocketImpl.cpp‎

Lines changed: 77 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//
22
// WebSocketImpl.cpp
33
//
4-
// $Id: //poco/1.4/Net/src/WebSocketImpl.cpp#2 $
4+
// $Id: //poco/1.4/Net/src/WebSocketImpl.cpp#6 $
55
//
66
// Library: Net
77
// Package: WebSocket
@@ -118,89 +118,92 @@ int WebSocketImpl::sendBytes(const void* buffer, int length, int flags)
118118
int WebSocketImpl::receiveBytes(void* buffer, int length, int)
119119
{
120120
char header[MAX_HEADER_LENGTH];
121-
int n = _pStreamSocketImpl->receiveBytes(header, 2);
122-
if (n == 1)
121+
int n = receiveNBytes(header, 2);
122+
poco_assert (n == 2);
123+
Poco::UInt8 lengthByte = static_cast<Poco::UInt8>(header[1]);
124+
int maskOffset = 0;
125+
if (lengthByte & FRAME_FLAG_MASK) maskOffset += 4;
126+
lengthByte &= 0x7f;
127+
if (lengthByte + 2 + maskOffset < MAX_HEADER_LENGTH)
123128
{
124-
n += _pStreamSocketImpl->receiveBytes(header + 1, 1);
129+
n = receiveNBytes(header + 2, lengthByte + maskOffset);
125130
}
126-
if (n == 2)
131+
else
127132
{
128-
Poco::UInt8 lengthByte = static_cast<Poco::UInt8>(header[1]) & 0x7f;
129-
if (lengthByte + 2 < MAX_HEADER_LENGTH)
130-
{
131-
n = _pStreamSocketImpl->receiveBytes(header + 2, lengthByte);
132-
}
133-
else
134-
{
135-
n = _pStreamSocketImpl->receiveBytes(header + 2, MAX_HEADER_LENGTH - 2);
136-
}
133+
n = receiveNBytes(header + 2, MAX_HEADER_LENGTH - 2);
137134
}
138-
else throw WebSocketException("Incomplete frame received", WebSocket::WS_ERR_INCOMPLETE_FRAME);
139135

140-
if (n > 0)
136+
poco_assert (n > 0);
137+
n += 2;
138+
Poco::MemoryInputStream istr(header, n);
139+
Poco::BinaryReader reader(istr, Poco::BinaryReader::NETWORK_BYTE_ORDER);
140+
Poco::UInt8 flags;
141+
char mask[4];
142+
reader >> flags >> lengthByte;
143+
_frameFlags = flags;
144+
int payloadLength = 0;
145+
int payloadOffset = 2;
146+
if ((lengthByte & 0x7f) == 127)
141147
{
142-
n += 2;
143-
Poco::MemoryInputStream istr(header, n);
144-
Poco::BinaryReader reader(istr, Poco::BinaryReader::NETWORK_BYTE_ORDER);
145-
Poco::UInt8 flags;
146-
Poco::UInt8 lengthByte;
147-
char mask[4];
148-
reader >> flags >> lengthByte;
149-
_frameFlags = flags;
150-
int payloadLength = 0;
151-
int payloadOffset = 2;
152-
if ((lengthByte & 0x7f) == 127)
153-
{
154-
Poco::UInt64 l;
155-
reader >> l;
156-
if (l > length) throw WebSocketException(Poco::format("Insufficient buffer for payload size %Lu", l), WebSocket::WS_ERR_PAYLOAD_TOO_BIG);
157-
payloadLength = static_cast<int>(l);
158-
payloadOffset += 8;
159-
}
160-
else if ((lengthByte & 0x7f) == 126)
148+
Poco::UInt64 l;
149+
reader >> l;
150+
if (l > length) throw WebSocketException(Poco::format("Insufficient buffer for payload size %Lu", l), WebSocket::WS_ERR_PAYLOAD_TOO_BIG);
151+
payloadLength = static_cast<int>(l);
152+
payloadOffset += 8;
153+
}
154+
else if ((lengthByte & 0x7f) == 126)
155+
{
156+
Poco::UInt16 l;
157+
reader >> l;
158+
if (l > length) throw WebSocketException(Poco::format("Insufficient buffer for payload size %hu", l), WebSocket::WS_ERR_PAYLOAD_TOO_BIG);
159+
payloadLength = static_cast<int>(l);
160+
payloadOffset += 2;
161+
}
162+
else
163+
{
164+
Poco::UInt8 l = lengthByte & 0x7f;
165+
if (l > length) throw WebSocketException(Poco::format("Insufficient buffer for payload size %u", unsigned(l)), WebSocket::WS_ERR_PAYLOAD_TOO_BIG);
166+
payloadLength = static_cast<int>(l);
167+
}
168+
if (lengthByte & FRAME_FLAG_MASK)
169+
{
170+
reader.readRaw(mask, 4);
171+
payloadOffset += 4;
172+
}
173+
int received = 0;
174+
if (payloadOffset < n)
175+
{
176+
std::memcpy(buffer, header + payloadOffset, n - payloadOffset);
177+
received = n - payloadOffset;
178+
}
179+
if (received < payloadLength)
180+
{
181+
received += receiveNBytes(reinterpret_cast<char*>(buffer) + received, payloadLength - received);
182+
}
183+
if (lengthByte & FRAME_FLAG_MASK)
184+
{
185+
char* p = reinterpret_cast<char*>(buffer);
186+
for (int i = 0; i < received; i++)
161187
{
162-
Poco::UInt16 l;
163-
reader >> l;
164-
if (l > length) throw WebSocketException(Poco::format("Insufficient buffer for payload size %hu", l), WebSocket::WS_ERR_PAYLOAD_TOO_BIG);
165-
payloadLength = static_cast<int>(l);
166-
payloadOffset += 2;
188+
p[i] ^= mask[i % 4];
167189
}
190+
}
191+
return received;
192+
}
193+
194+
195+
int WebSocketImpl::receiveNBytes(void* buffer, int bytes)
196+
{
197+
int received = _pStreamSocketImpl->receiveBytes(reinterpret_cast<char*>(buffer), bytes);
198+
while (received < bytes)
199+
{
200+
int n = _pStreamSocketImpl->receiveBytes(reinterpret_cast<char*>(buffer) + received, bytes - received);
201+
if (n > 0)
202+
received += n;
168203
else
169-
{
170-
Poco::UInt8 l = lengthByte & 0x7f;
171-
if (l > length) throw WebSocketException(Poco::format("Insufficient buffer for payload size %u", unsigned(l)), WebSocket::WS_ERR_PAYLOAD_TOO_BIG);
172-
payloadLength = static_cast<int>(l);
173-
}
174-
if (lengthByte & FRAME_FLAG_MASK)
175-
{
176-
reader.readRaw(mask, 4);
177-
payloadOffset += 4;
178-
}
179-
int received = 0;
180-
if (payloadOffset < n)
181-
{
182-
std::memcpy(buffer, header + payloadOffset, n - payloadOffset);
183-
received = n - payloadOffset;
184-
}
185-
while (received < payloadLength)
186-
{
187-
n = _pStreamSocketImpl->receiveBytes(reinterpret_cast<char*>(buffer) + received, payloadLength - received);
188-
if (n > 0)
189-
received += n;
190-
else
191-
throw WebSocketException("Incomplete frame received", WebSocket::WS_ERR_INCOMPLETE_FRAME);
192-
}
193-
if (lengthByte & FRAME_FLAG_MASK)
194-
{
195-
char* p = reinterpret_cast<char*>(buffer);
196-
for (int i = 0; i < received; i++)
197-
{
198-
p[i] ^= mask[i % 4];
199-
}
200-
}
201-
return received;
204+
throw WebSocketException("Incomplete frame received", WebSocket::WS_ERR_INCOMPLETE_FRAME);
202205
}
203-
return n;
206+
return received;
204207
}
205208

206209

‎Net/testsuite/src/WebSocketTest.cpp‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//
22
// WebSocketTest.cpp
33
//
4-
// $Id: //poco/1.4/Net/testsuite/src/WebSocketTest.cpp#2 $
4+
// $Id: //poco/1.4/Net/testsuite/src/WebSocketTest.cpp#3 $
55
//
66
// Copyright (c) 2012, Applied Informatics Software Engineering GmbH.
77
// and Contributors.
@@ -106,6 +106,16 @@ namespace
106106

107107

108108
WebSocketTest::WebSocketTest(const std::string& name): CppUnit::TestCase(name)
109+
{
110+
}
111+
112+
113+
WebSocketTest::~WebSocketTest()
114+
{
115+
}
116+
117+
118+
void WebSocketTest::testWebSocket()
109119
{
110120
Poco::Net::ServerSocket ss(0);
111121
Poco::Net::HTTPServer server(new WebSocketRequestHandlerFactory, ss, new Poco::Net::HTTPServerParams);
@@ -117,14 +127,42 @@ WebSocketTest::WebSocketTest(const std::string& name): CppUnit::TestCase(name)
117127
HTTPRequest request(HTTPRequest::HTTP_GET, "/ws");
118128
HTTPResponse response;
119129
WebSocket ws(cs, request, response);
120-
std::string payload("Hello, world!");
130+
131+
std::string payload("x");
121132
ws.sendFrame(payload.data(), payload.size());
122133
char buffer[1024];
123134
int flags;
124135
int n = ws.receiveFrame(buffer, sizeof(buffer), flags);
125136
assert (n == payload.size());
126137
assert (payload.compare(0, payload.size(), buffer, 0, n) == 0);
127138
assert (flags == WebSocket::FRAME_TEXT);
139+
140+
for (int i = 2; i < 20; i++)
141+
{
142+
payload.assign(i, 'x');
143+
ws.sendFrame(payload.data(), payload.size());
144+
n = ws.receiveFrame(buffer, sizeof(buffer), flags);
145+
assert (n == payload.size());
146+
assert (payload.compare(0, payload.size(), buffer, 0, n) == 0);
147+
assert (flags == WebSocket::FRAME_TEXT);
148+
}
149+
150+
for (int i = 125; i < 129; i++)
151+
{
152+
payload.assign(i, 'x');
153+
ws.sendFrame(payload.data(), payload.size());
154+
n = ws.receiveFrame(buffer, sizeof(buffer), flags);
155+
assert (n == payload.size());
156+
assert (payload.compare(0, payload.size(), buffer, 0, n) == 0);
157+
assert (flags == WebSocket::FRAME_TEXT);
158+
}
159+
160+
payload = "Hello, world!";
161+
ws.sendFrame(payload.data(), payload.size());
162+
n = ws.receiveFrame(buffer, sizeof(buffer), flags);
163+
assert (n == payload.size());
164+
assert (payload.compare(0, payload.size(), buffer, 0, n) == 0);
165+
assert (flags == WebSocket::FRAME_TEXT);
128166

129167
payload = "Hello, universe!";
130168
ws.sendFrame(payload.data(), payload.size(), WebSocket::FRAME_BINARY);
@@ -142,16 +180,6 @@ WebSocketTest::WebSocketTest(const std::string& name): CppUnit::TestCase(name)
142180
}
143181

144182

145-
WebSocketTest::~WebSocketTest()
146-
{
147-
}
148-
149-
150-
void WebSocketTest::testWebSocket()
151-
{
152-
}
153-
154-
155183
void WebSocketTest::setUp()
156184
{
157185
}

0 commit comments

Comments
 (0)