From c0dac220de0186a879f1f71966a2848000f69a48 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 21 May 2020 12:11:53 +0200 Subject: [PATCH] Remove readString()/writeString() These are not universal in the protocol so having functions for them only obfuscates things. --- common/rdr/CMakeLists.txt | 1 - common/rdr/InStream.cxx | 35 ----------------------------------- common/rdr/InStream.h | 11 ----------- common/rdr/OutStream.h | 10 ---------- common/rfb/CConnection.cxx | 11 ++++++++--- common/rfb/CMsgReader.cxx | 14 +++++++++----- common/rfb/SConnection.cxx | 21 +++++++++++++-------- common/rfb/SMsgWriter.cxx | 6 ++++-- common/rfb/VNCServerST.cxx | 4 +++- 9 files changed, 37 insertions(+), 76 deletions(-) delete mode 100644 common/rdr/InStream.cxx diff --git a/common/rdr/CMakeLists.txt b/common/rdr/CMakeLists.txt index 78778ddc..fa6ca281 100644 --- a/common/rdr/CMakeLists.txt +++ b/common/rdr/CMakeLists.txt @@ -9,7 +9,6 @@ add_library(rdr STATIC FileInStream.cxx HexInStream.cxx HexOutStream.cxx - InStream.cxx RandomStream.cxx TLSException.cxx TLSInStream.cxx diff --git a/common/rdr/InStream.cxx b/common/rdr/InStream.cxx deleted file mode 100644 index a413b6c1..00000000 --- a/common/rdr/InStream.cxx +++ /dev/null @@ -1,35 +0,0 @@ -/* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * - * This is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This software is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this software; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, - * USA. - */ - -#include -#include - -using namespace rdr; - -U32 InStream::maxStringLength = 65535; - -char* InStream::readString() -{ - U32 len = readU32(); - if (len > maxStringLength) - throw Exception("InStream max string length exceeded"); - char* str = new char[len+1]; - readBytes(str, len); - str[len] = 0; - return str; -} diff --git a/common/rdr/InStream.h b/common/rdr/InStream.h index 46af74c9..5d873011 100644 --- a/common/rdr/InStream.h +++ b/common/rdr/InStream.h @@ -76,17 +76,6 @@ namespace rdr { inline S16 readS16() { return (S16)readU16(); } inline S32 readS32() { return (S32)readU32(); } - // readString() reads a string - a U32 length followed by the data. - // Returns a null-terminated string - the caller should delete[] it - // afterwards. - - char* readString(); - - // maxStringLength protects against allocating a huge buffer. Set it - // higher if you need longer strings. - - static U32 maxStringLength; - inline void skip(size_t bytes) { while (bytes > 0) { size_t n = check(1, bytes); diff --git a/common/rdr/OutStream.h b/common/rdr/OutStream.h index 0d5f113e..f432520f 100644 --- a/common/rdr/OutStream.h +++ b/common/rdr/OutStream.h @@ -68,16 +68,6 @@ namespace rdr { inline void writeS16(S16 s) { writeU16((U16)s); } inline void writeS32(S32 s) { writeU32((U32)s); } - // writeString() writes a string - a U32 length followed by the data. The - // given string should be null-terminated (but the terminating null is not - // written to the stream). - - inline void writeString(const char* str) { - U32 len = strlen(str); - writeU32(len); - writeBytes(str, len); - } - inline void pad(size_t bytes) { while (bytes-- > 0) writeU8(0); } diff --git a/common/rfb/CConnection.cxx b/common/rfb/CConnection.cxx index f541a969..cb1b84bd 100644 --- a/common/rfb/CConnection.cxx +++ b/common/rfb/CConnection.cxx @@ -301,7 +301,10 @@ void CConnection::processSecurityResultMsg() state_ = RFBSTATE_INVALID; if (server.beforeVersion(3,8)) throw AuthFailureException(); - CharArray reason(is->readString()); + rdr::U32 len = is->readU32(); + CharArray reason(len + 1); + is->readBytes(reason.buf, len); + reason.buf[len] = '\0'; throw AuthFailureException(reason.buf); } @@ -314,8 +317,10 @@ void CConnection::processInitMsg() void CConnection::throwConnFailedException() { state_ = RFBSTATE_INVALID; - CharArray reason; - reason.buf = is->readString(); + rdr::U32 len = is->readU32(); + CharArray reason(len + 1); + is->readBytes(reason.buf, len); + reason.buf[len] = '\0'; throw ConnFailedException(reason.buf); } diff --git a/common/rfb/CMsgReader.cxx b/common/rfb/CMsgReader.cxx index 52d40ce7..a015ec99 100644 --- a/common/rfb/CMsgReader.cxx +++ b/common/rfb/CMsgReader.cxx @@ -53,7 +53,10 @@ void CMsgReader::readServerInit() int height = is->readU16(); PixelFormat pf; pf.read(is); - CharArray name(is->readString()); + rdr::U32 len = is->readU32(); + CharArray name(len + 1); + is->readBytes(name.buf, len); + name.buf[len] = '\0'; handler->serverInit(width, height, pf, name.buf); } @@ -556,15 +559,16 @@ void CMsgReader::readSetVMwareCursor(int width, int height, const Point& hotspot void CMsgReader::readSetDesktopName(int x, int y, int w, int h) { - char* name = is->readString(); + rdr::U32 len = is->readU32(); + CharArray name(len + 1); + is->readBytes(name.buf, len); + name.buf[len] = '\0'; if (x || y || w || h) { vlog.error("Ignoring DesktopName rect with non-zero position/size"); } else { - handler->setName(name); + handler->setName(name.buf); } - - delete [] name; } void CMsgReader::readExtendedDesktopSize(int x, int y, int w, int h) diff --git a/common/rfb/SConnection.cxx b/common/rfb/SConnection.cxx index b5a69d4c..e06fc6bb 100644 --- a/common/rfb/SConnection.cxx +++ b/common/rfb/SConnection.cxx @@ -268,8 +268,11 @@ bool SConnection::handleAuthFailureTimeout(Timer* t) try { os->writeU32(secResultFailed); - if (!client.beforeVersion(3,8)) // 3.8 onwards have failure message - os->writeString(authFailureMsg.buf); + if (!client.beforeVersion(3,8)) { // 3.8 onwards have failure message + const char* reason = authFailureMsg.buf; + os->writeU32(strlen(reason)); + os->writeBytes(reason, strlen(reason)); + } os->flush(); } catch (rdr::Exception& e) { close(e.str()); @@ -295,11 +298,13 @@ void SConnection::throwConnFailedException(const char* format, ...) if (state_ == RFBSTATE_PROTOCOL_VERSION) { if (client.majorVersion == 3 && client.minorVersion == 3) { os->writeU32(0); - os->writeString(str); + os->writeU32(strlen(str)); + os->writeBytes(str, strlen(str)); os->flush(); } else { os->writeU8(0); - os->writeString(str); + os->writeU32(strlen(str)); + os->writeBytes(str, strlen(str)); os->flush(); } } @@ -425,10 +430,10 @@ void SConnection::approveConnection(bool accept, const char* reason) } else { os->writeU32(secResultFailed); if (!client.beforeVersion(3,8)) { // 3.8 onwards have failure message - if (reason) - os->writeString(reason); - else - os->writeString("Authentication failure"); + if (!reason) + reason = "Authentication failure"; + os->writeU32(strlen(reason)); + os->writeBytes(reason, strlen(reason)); } } os->flush(); diff --git a/common/rfb/SMsgWriter.cxx b/common/rfb/SMsgWriter.cxx index becf6e70..a29ed9d8 100644 --- a/common/rfb/SMsgWriter.cxx +++ b/common/rfb/SMsgWriter.cxx @@ -56,7 +56,8 @@ void SMsgWriter::writeServerInit(rdr::U16 width, rdr::U16 height, os->writeU16(width); os->writeU16(height); pf.write(os); - os->writeString(name); + os->writeU32(strlen(name)); + os->writeBytes(name, strlen(name)); endMsg(); } @@ -551,7 +552,8 @@ void SMsgWriter::writeSetDesktopNameRect(const char *name) os->writeU16(0); os->writeU16(0); os->writeU32(pseudoEncodingDesktopName); - os->writeString(name); + os->writeU32(strlen(name)); + os->writeBytes(name, strlen(name)); } void SMsgWriter::writeSetCursorRect(int width, int height, diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index 8a005334..be1662d8 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -137,7 +137,9 @@ void VNCServerST::addSocket(network::Socket* sock, bool outgoing) // Shortest possible way to tell a client it is not welcome os.writeBytes("RFB 003.003\n", 12); os.writeU32(0); - os.writeString("Too many security failures"); + const char* reason = "Too many security failures"; + os.writeU32(strlen(reason)); + os.writeBytes(reason, strlen(reason)); os.flush(); } catch (rdr::Exception&) { } -- 2.39.5