diff --git a/src/vnc_session_backend.cpp b/src/vnc_session_backend.cpp index 6be3304..035068d 100644 --- a/src/vnc_session_backend.cpp +++ b/src/vnc_session_backend.cpp @@ -8,10 +8,33 @@ #include +#include #include #include namespace { +// RFB (RFC 6143) message-type constants, client -> server. +constexpr char kMsgSetPixelFormat = 0; +constexpr char kMsgSetEncodings = 2; +constexpr char kMsgFramebufferUpdateRequest = 3; +constexpr char kMsgKeyEvent = 4; +constexpr char kMsgPointerEvent = 5; +constexpr char kMsgClientCutText = 6; + +// RFB message-type constants, server -> client. +constexpr quint8 kServerMsgFramebufferUpdate = 0; +constexpr quint8 kServerMsgSetColourMapEntries = 1; +constexpr quint8 kServerMsgBell = 2; +constexpr quint8 kServerMsgServerCutText = 3; + +// Encoding types. This list grows as new FramebufferUpdate rectangle +// encodings are supported; kAnnouncedEncodings below is the single source +// of truth for what we tell the server we can decode via SetEncodings. +constexpr qint32 kEncRaw = 0; +constexpr qint32 kEncCopyRect = 1; + +constexpr std::array kAnnouncedEncodings = { kEncRaw, kEncCopyRect }; + quint16 readU16BE(const QByteArray& buf, int offset) { const auto* p = reinterpret_cast(buf.constData()) + offset; @@ -173,7 +196,7 @@ void VncSessionBackend::sendKeyEvent(int key, } QByteArray msg; - msg.append(char(4)); // message-type: KeyEvent + msg.append(kMsgKeyEvent); msg.append(pressed ? char(1) : char(0)); appendU16BE(msg, 0); // padding appendU32BE(msg, keysym); @@ -368,7 +391,7 @@ void VncSessionBackend::sendSetPixelFormatAndEncodings() // wrapped with zero conversion -- the same trick // RdpSessionBackend::orbitEndPaint uses for FreeRDP's GDI buffer. QByteArray setPixelFormat; - setPixelFormat.append(char(0)); // message-type: SetPixelFormat + setPixelFormat.append(kMsgSetPixelFormat); setPixelFormat.append(3, char(0)); // padding setPixelFormat.append(char(32)); // bits-per-pixel setPixelFormat.append(char(24)); // depth @@ -384,18 +407,19 @@ void VncSessionBackend::sendSetPixelFormatAndEncodings() m_socket->write(setPixelFormat); QByteArray setEncodings; - setEncodings.append(char(2)); // message-type: SetEncodings + setEncodings.append(kMsgSetEncodings); setEncodings.append(char(0)); // padding - appendU16BE(setEncodings, 2); // number-of-encodings - appendU32BE(setEncodings, 0); // Raw - appendU32BE(setEncodings, 1); // CopyRect + appendU16BE(setEncodings, static_cast(kAnnouncedEncodings.size())); + for (qint32 encoding : kAnnouncedEncodings) { + appendU32BE(setEncodings, static_cast(encoding)); + } m_socket->write(setEncodings); } void VncSessionBackend::requestFramebufferUpdate(bool incremental) { QByteArray msg; - msg.append(char(3)); // message-type: FramebufferUpdateRequest + msg.append(kMsgFramebufferUpdateRequest); msg.append(incremental ? char(1) : char(0)); appendU16BE(msg, 0); // x appendU16BE(msg, 0); // y @@ -410,7 +434,7 @@ void VncSessionBackend::sendPointerEvent() return; } QByteArray msg; - msg.append(char(5)); // message-type: PointerEvent + msg.append(kMsgPointerEvent); msg.append(static_cast(m_pointerButtonMask)); appendU16BE(msg, static_cast(qBound(0, m_lastPointerX, 65535))); appendU16BE(msg, static_cast(qBound(0, m_lastPointerY, 65535))); @@ -427,14 +451,14 @@ void VncSessionBackend::sendWheelClick(quint8 wheelBit) const quint16 y = static_cast(qBound(0, m_lastPointerY, 65535)); QByteArray press; - press.append(char(5)); + press.append(kMsgPointerEvent); press.append(static_cast(m_pointerButtonMask | wheelBit)); appendU16BE(press, x); appendU16BE(press, y); m_socket->write(press); QByteArray release; - release.append(char(5)); + release.append(kMsgPointerEvent); release.append(static_cast(m_pointerButtonMask)); appendU16BE(release, x); appendU16BE(release, y); @@ -706,17 +730,17 @@ void VncSessionBackend::processReceiveBuffer() const quint8 messageType = static_cast(m_recvBuffer.at(0)); m_recvBuffer.remove(0, 1); switch (messageType) { - case 0: + case kServerMsgFramebufferUpdate: m_rfbState = RfbState::WaitingFramebufferUpdateHeader; break; - case 1: + case kServerMsgSetColourMapEntries: m_rfbState = RfbState::WaitingSetColourMapHeader; break; - case 2: + case kServerMsgBell: // Bell -- nothing to render; stay in the same state. emit eventLogged(QStringLiteral("Remote bell.")); break; - case 3: + case kServerMsgServerCutText: m_rfbState = RfbState::WaitingServerCutTextHeader; break; default: @@ -756,16 +780,36 @@ void VncSessionBackend::processReceiveBuffer() m_currentRectangle.encoding = static_cast(readU32BE(m_recvBuffer, 8)); m_recvBuffer.remove(0, 12); - if (m_currentRectangle.encoding == 0) { + switch (m_currentRectangle.encoding) { + case kEncRaw: m_rfbState = RfbState::WaitingRawPixelData; - } else if (m_currentRectangle.encoding == 1) { + break; + case kEncCopyRect: m_rfbState = RfbState::WaitingCopyRectSource; - } else { + break; + default: { + // SetEncodings (see kAnnouncedEncodings) is entirely + // client-controlled, so a spec-compliant server will never + // send an encoding we didn't announce. Reaching here means + // either a non-compliant server, or -- more likely in + // practice -- an OrbitHub bug where an encoding was added to + // kAnnouncedEncodings without a matching case above. There's + // no safe way to skip an unrecognized rectangle's payload + // (its length depends on decoding it), so this must stay + // fatal rather than attempt to guess and resync. + QStringList announced; + for (qint32 encoding : kAnnouncedEncodings) { + announced.push_back(QString::number(encoding)); + } failConnection( - QStringLiteral("The VNC server sent an encoding OrbitHub doesn't support."), - QStringLiteral("Encoding type %1").arg(m_currentRectangle.encoding)); + QStringLiteral("The VNC server sent an encoding it was never offered -- " + "this indicates a non-compliant server."), + QStringLiteral("Encoding type %1 (announced: %2)") + .arg(m_currentRectangle.encoding) + .arg(announced.join(QStringLiteral(", ")))); return; } + } break; } diff --git a/tests/test_vnc_session_backend.cpp b/tests/test_vnc_session_backend.cpp index 958f03a..58a2536 100644 --- a/tests/test_vnc_session_backend.cpp +++ b/tests/test_vnc_session_backend.cpp @@ -108,6 +108,8 @@ private slots: void unsupportedSecurityTypeReachesFailedState(); void rfb33ServerWithNoAuthConnectsWithoutSecurityResult(); void rawFramebufferUpdateProducesExpectedPixels(); + void copyRectEncodingDoesNotFailConnection(); + void unannouncedEncodingFailsConnectionWithClearMessage(); private: std::unique_ptr m_server; @@ -491,5 +493,132 @@ void TestVncSessionBackend::rawFramebufferUpdateProducesExpectedPixels() QCOMPARE(m_lastFrame.pixelColor(1, 0), QColor(0, 255, 0)); } +// Regression guard for the WaitingRectangleHeader dispatch: every encoding +// this backend announces via SetEncodings must have a working decode path. +// Raw is already exercised by rawFramebufferUpdateProducesExpectedPixels(); +// this covers CopyRect. Whenever a new encoding is added to +// kAnnouncedEncodings in vnc_session_backend.cpp, a matching test belongs +// here (or nearby) so the announced list and the dispatch switch can never +// silently drift apart. +void TestVncSessionBackend::copyRectEncodingDoesNotFailConnection() +{ + int frameCount = 0; + connect(m_backend.get(), &SessionBackend::frameUpdated, this, + [&frameCount](const QImage&) { ++frameCount; }); + + connect(m_server.get(), &FakeVncServer::clientConnected, this, [this]() { + m_server->sendWhenConnected(QByteArray("RFB 003.008\n")); + }); + connect(m_server.get(), &FakeVncServer::dataReceived, this, [this]() { + m_server->received.clear(); + switch (m_server->nextStep()) { + case 0: { + QByteArray securityTypes; + securityTypes.append(char(1)); + securityTypes.append(char(1)); + m_server->sendWhenConnected(securityTypes); + break; + } + case 1: + m_server->sendWhenConnected(QByteArray(4, char(0))); // SecurityResult: OK + break; + case 2: { // ClientInit + QByteArray serverInit; + serverInit.append(char(0)); serverInit.append(char(2)); // width = 2 + serverInit.append(char(0)); serverInit.append(char(2)); // height = 2 + serverInit.append(QByteArray(16, char(0))); + serverInit.append(QByteArray(4, char(0))); + m_server->sendWhenConnected(serverInit); + break; + } + case 3: { // initial non-incremental request -> seed the framebuffer with Raw + QByteArray update; + update.append(char(0)); update.append(char(0)); + update.append(char(0)); update.append(char(1)); // 1 rectangle + update.append(char(0)); update.append(char(0)); // x + update.append(char(0)); update.append(char(0)); // y + update.append(char(0)); update.append(char(2)); // width + update.append(char(0)); update.append(char(2)); // height + update.append(char(0)); update.append(char(0)); update.append(char(0)); + update.append(char(0)); // encoding = Raw + update.append(QByteArray(2 * 2 * 4, char(0x11))); + m_server->sendWhenConnected(update); + break; + } + case 4: { // next incremental request -> a CopyRect rectangle + QByteArray update; + update.append(char(0)); update.append(char(0)); + update.append(char(0)); update.append(char(1)); // 1 rectangle + update.append(char(0)); update.append(char(0)); // x + update.append(char(0)); update.append(char(0)); // y + update.append(char(0)); update.append(char(2)); // width + update.append(char(0)); update.append(char(2)); // height + update.append(char(0)); update.append(char(0)); update.append(char(0)); + update.append(char(1)); // encoding = CopyRect + update.append(char(0)); update.append(char(0)); // src x = 0 + update.append(char(0)); update.append(char(0)); // src y = 0 + m_server->sendWhenConnected(update); + break; + } + default: + break; + } + }); + + m_backend->connectSession(makeOptions()); + QTRY_VERIFY(frameCount >= 2); + QCOMPARE(m_lastState, SessionState::Connected); +} + +void TestVncSessionBackend::unannouncedEncodingFailsConnectionWithClearMessage() +{ + connect(m_server.get(), &FakeVncServer::clientConnected, this, [this]() { + m_server->sendWhenConnected(QByteArray("RFB 003.008\n")); + }); + connect(m_server.get(), &FakeVncServer::dataReceived, this, [this]() { + m_server->received.clear(); + switch (m_server->nextStep()) { + case 0: { + QByteArray securityTypes; + securityTypes.append(char(1)); + securityTypes.append(char(1)); + m_server->sendWhenConnected(securityTypes); + break; + } + case 1: + m_server->sendWhenConnected(QByteArray(4, char(0))); // SecurityResult: OK + break; + case 2: { // ClientInit + QByteArray serverInit; + serverInit.append(char(0)); serverInit.append(char(1)); // width = 1 + serverInit.append(char(0)); serverInit.append(char(1)); // height = 1 + serverInit.append(QByteArray(16, char(0))); + serverInit.append(QByteArray(4, char(0))); + m_server->sendWhenConnected(serverInit); + break; + } + case 3: { // a rectangle whose encoding was never announced via SetEncodings + QByteArray update; + update.append(char(0)); update.append(char(0)); + update.append(char(0)); update.append(char(1)); // 1 rectangle + update.append(char(0)); update.append(char(0)); // x + update.append(char(0)); update.append(char(0)); // y + update.append(char(0)); update.append(char(1)); // width + update.append(char(0)); update.append(char(1)); // height + update.append(char(0)); update.append(char(0)); update.append(char(0)); + update.append(char(99)); // encoding = 99, never announced + m_server->sendWhenConnected(update); + break; + } + default: + break; + } + }); + + m_backend->connectSession(makeOptions()); + QTRY_COMPARE(m_lastState, SessionState::Failed); + QVERIFY(m_lastErrorDisplay.contains(QStringLiteral("non-compliant"))); +} + QTEST_GUILESS_MAIN(TestVncSessionBackend) #include "test_vnc_session_backend.moc"