[wx-dev] IPC issues
Steven Van Ingelgem
steven at vaningelgem.be
Sun Dec 2 01:04:20 PST 2007
Hey Brian,
What I spotted immediately is how unsafe it becomes now. Before you
could do an Execute from a void*. I don't know how binary safe
wxString is, but I imaging people wrapping their void* calls into
wxString(char*, size_t len) calls.
Thus you will get a binary string, which would result in a wrong
length at wxStrlen... Personally I think it's better to use
wxString.Length() for this.
All depending on the binary-safety of wxString. (iirc it's not).
Greetings,
Steven
On 12/2/07, Brian Vanderburg II <BrianVanderburg2 at aim.com> wrote:
> This is just another possible idea. It seems that in IPC, the data from
> one end to the other should remain in the same format (no conversions or
> changes to the data should take place). This is not a problem for TCP.
> It doesn't seem like it should be a problem for DDE except for Execute,
> but seems like the rest should be fine, that is, if an application pokes
> data as wxIPC_TEXT windows DDE doesn't seem like it does anything to it
> and sends it to the server as wxIPC_TEXT.
>
> So since Execute seems to be the only problem, perhaps it should be
> changed to only support command strings. The remaining functions can be
> kept as is, where char* will default to wxIPC_TEXT, and wchar_T will be
> wxIPC_UNICODETEXT, or the application can if sending text convert to
> utf8 and use the void* version of the other functions. Execute should
> take only wxString
>
> I've attached a patch illustrating this idea a little better.
>
>
> Brian Vanderburg II
>
>
> Index: include/wx/ipcbase.h
> ===================================================================
> --- include/wx/ipcbase.h (revision 50401)
> +++ include/wx/ipcbase.h (working copy)
> @@ -62,21 +62,8 @@
> bool GetConnected() { return m_connected; }
>
> // Calls that CLIENT can make
> - bool Execute(const void *data, size_t size, wxIPCFormat fmt = wxIPC_PRIVATE)
> - { return DoExecute(data, size, fmt); }
> - bool Execute(const char *s, size_t size = wxNO_LEN)
> - { return DoExecute(s, size == wxNO_LEN ? strlen(s) + 1
> - : size, wxIPC_TEXT); }
> - bool Execute(const wchar_t *ws, size_t size = wxNO_LEN)
> - { return DoExecute(ws, size == wxNO_LEN ? (wcslen(ws) + 1)*sizeof(wchar_t)
> - : size, wxIPC_UNICODETEXT); }
> - bool Execute(const wxString& s)
> - {
> - const wxUTF8Buf buf = s.utf8_str();
> - return DoExecute(buf, strlen(buf) + 1, wxIPC_UTF8TEXT);
> - }
> - bool Execute(const wxCStrData& cs)
> - { return Execute(cs.AsString()); }
> + bool Execute(const wxString& command)
> + { return DoExecute(command); }
>
> virtual const void *Request(const wxString& item,
> size_t *size = NULL,
> @@ -128,9 +115,7 @@
>
> // Callbacks to SERVER - override at will
> virtual bool OnExecute(const wxString& WXUNUSED(topic),
> - const void *WXUNUSED(data),
> - size_t WXUNUSED(size),
> - wxIPCFormat WXUNUSED(format))
> + const wxString& WXUNUSED(command))
> { return false; }
>
> virtual const void *OnRequest(const wxString& WXUNUSED(topic),
> @@ -165,13 +150,12 @@
> // Callbacks to BOTH
> virtual bool OnDisconnect() { delete this; return true; }
>
> -
> // return a buffer at least this size, reallocating buffer if needed
> // returns NULL if using an inadequate user buffer which can't be resized
> void *GetBufferAtLeast(size_t bytes);
>
> protected:
> - virtual bool DoExecute(const void *data, size_t size, wxIPCFormat format) = 0;
> + virtual bool DoExecute(const wxString& command) = 0;
> virtual bool DoPoke(const wxString& item, const void *data, size_t size,
> wxIPCFormat format) = 0;
> virtual bool DoAdvise(const wxString& item, const void *data, size_t size,
> Index: include/wx/msw/dde.h
> ===================================================================
> --- include/wx/msw/dde.h (revision 50401)
> +++ include/wx/msw/dde.h (working copy)
> @@ -57,7 +57,7 @@
> virtual bool Disconnect();
>
> protected:
> - virtual bool DoExecute(const void *data, size_t size, wxIPCFormat format);
> + virtual bool DoExecute(const wxString& command);
> virtual bool DoPoke(const wxString& item, const void *data, size_t size,
> wxIPCFormat format);
> virtual bool DoAdvise(const wxString& item, const void *data, size_t size,
> Index: include/wx/sckipc.h
> ===================================================================
> --- include/wx/sckipc.h (revision 50401)
> +++ include/wx/sckipc.h (working copy)
> @@ -72,7 +72,7 @@
> virtual bool Disconnect(void);
>
> protected:
> - virtual bool DoExecute(const void *data, size_t size, wxIPCFormat format);
> + virtual bool DoExecute(const wxString& command);
> virtual bool DoPoke(const wxString& item, const void *data, size_t size,
> wxIPCFormat format);
> virtual bool DoAdvise(const wxString& item, const void *data, size_t size,
> Index: src/common/sckipc.cpp
> ===================================================================
> --- src/common/sckipc.cpp (revision 50401)
> +++ src/common/sckipc.cpp (working copy)
> @@ -393,17 +393,19 @@
> return true;
> }
>
> -bool wxTCPConnection::DoExecute(const void *data, size_t size, wxIPCFormat format)
> +bool wxTCPConnection::DoExecute(const wxString& command)
> {
> if (!m_sock->IsConnected())
> return false;
>
> + wxUTF8Buf buffer = command.utf8_str();
> + size_t size = strlen(buffer) + 1;
> +
> // Prepare EXECUTE message
> m_codeco->Write8(IPC_EXECUTE);
> - m_codeco->Write8(format);
>
> m_codeco->Write32(size);
> - m_sockstrm->Write(data, size);
> + m_sockstrm->Write((const void*)(const char*)data, size);
>
> return true;
> }
> @@ -555,17 +557,17 @@
> {
> void *data;
> size_t size;
> - wxIPCFormat format;
>
> - format = (wxIPCFormat)codeci->Read8();
> size = codeci->Read32();
>
> data = connection->GetBufferAtLeast( size );
> wxASSERT_MSG(data != NULL,
> _T("Buffer too small in wxTCPEventHandler::Client_OnRequest") );
> sockstrm->Read(data, size);
> +
> + wxString cmd((char*)data, wxConvUTF8, size - 1); // don't embed null terminator into string
>
> - connection->OnExecute (topic_name, data, size, format);
> + connection->OnExecute (cmd);
>
> break;
> }
> Index: src/msw/dde.cpp
> ===================================================================
> --- src/msw/dde.cpp (revision 50401)
> +++ src/msw/dde.cpp (working copy)
> @@ -548,94 +548,20 @@
> }
>
> bool
> -wxDDEConnection::DoExecute(const void *data, size_t size, wxIPCFormat format)
> +wxDDEConnection::DoExecute(const wxString& command)
> {
> - wxCHECK_MSG( format == wxIPC_TEXT ||
> - format == wxIPC_UTF8TEXT ||
> - format == wxIPC_UNICODETEXT,
> - false,
> - _T("wxDDEServer::Execute() supports only text data") );
> -
> - wxMemoryBuffer buffer;
> - LPBYTE realData wxDUMMY_INITIALIZE(NULL);
> - size_t realSize wxDUMMY_INITIALIZE(0);
> - wxMBConv *conv = NULL;
> -
> - // Windows only supports either ANSI or UTF-16 format depending on the
> - // build, so we need to convert the data if it doesn't use it already
> + // Convert data to the correct format
> + wxCStrData data = command.c_str();
> #if wxUSE_UNICODE
> - if ( format == wxIPC_TEXT )
> - {
> - conv = &wxConvLibc;
> - }
> - else if ( format == wxIPC_UTF8TEXT )
> - {
> - conv = &wxConvUTF8;
> - }
> - else // no conversion necessary for wxIPC_UNICODETEXT
> - {
> - realData = (LPBYTE)data;
> - realSize = size;
> - }
> + const wchar_t* realData = (const wchar_t*)data;
> +#else
> + const char* realData = (const char*)data;
> +#endif
> + size_t realSize = wxStrlen(realData) + 1;
> +
>
> - if ( conv )
> - {
> - const char * const text = (const char *)data;
> - const size_t len = size/sizeof(char);
> -
> - realSize = conv->ToWChar(NULL, 0, text, len);
> - if ( realSize == wxCONV_FAILED )
> - return false;
> -
> - realData = (LPBYTE)buffer.GetWriteBuf(realSize*sizeof(wchar_t));
> - if ( !realData )
> - return false;
> -
> - realSize = conv->ToWChar((wchar_t *)realData, realSize, text, len);
> - if ( realSize == wxCONV_FAILED )
> - return false;
> - }
> -#else // !wxUSE_UNICODE
> - if ( format == wxIPC_UNICODETEXT )
> - {
> - conv = &wxConvLibc;
> - }
> - else if ( format == wxIPC_UTF8TEXT )
> - {
> - // we could implement this in theory but it's not obvious how to pass
> - // the format information and, basically, why bother -- just use
> - // Unicode build
> - wxFAIL_MSG( _T("UTF-8 text not supported in ANSI build") );
> -
> - return false;
> - }
> - else // don't convert wxIPC_TEXT
> - {
> - realData = (LPBYTE)data;
> - realSize = size;
> - }
> -
> - if ( conv )
> - {
> - const wchar_t * const wtext = (const wchar_t *)data;
> - const size_t len = size/sizeof(wchar_t);
> -
> - realSize = conv->FromWChar(NULL, 0, wtext, len);
> - if ( realSize == wxCONV_FAILED )
> - return false;
> -
> - realData = (LPBYTE)buffer.GetWriteBuf(realSize*sizeof(char));
> - if ( !realData )
> - return false;
> -
> - realSize = conv->FromWChar((char*)realData, realSize, wtext, len);
> - if ( realSize == wxCONV_FAILED )
> - return false;
> - }
> -#endif // wxUSE_UNICODE/!wxUSE_UNICODE
> -
> DWORD result;
> - bool ok = DdeClientTransaction(realData,
> + bool ok = DdeClientTransaction((LPBYTE)realData,
> realSize,
> GetHConv(),
> NULL,
> @@ -854,16 +780,26 @@
> // XTYP_EXECUTE can be used for text only and the text is
> // always in ANSI format for ANSI build and Unicode format
> // in Unicode build
> + wxString cmd = wxEmptyString;
> +
> #if wxUSE_UNICODE
> - wFmt = wxIPC_UNICODETEXT;
> + if(len >= sizeof(wchar_t))
> + {
> + len /= sizeof(wchar_t); // convert to characters
> + len -= 1; // don't embed null terminator
> + cmd = wxString((wchar_t*)data, (size_t)len);
> + }
> #else
> - wFmt = wxIPC_TEXT;
> + if(len >= sizeof(char))
> + {
> + len /= sizeof(char); // convert to characters
> + len -= 1; // don't embed null terminator
> + cmd = wxString((char*)data, (size_t)len);
> + }
> #endif
> -
> - if ( connection->OnExecute(connection->m_topicName,
> - data,
> - (int)len,
> - (wxIPCFormat)wFmt) )
> +
> +
> + if ( connection->OnExecute(cmd) )
> {
> return (DDERETURN)(DWORD)DDE_FACK;
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: wx-dev-unsubscribe at lists.wxwidgets.org
> For additional commands, e-mail: wx-dev-help at lists.wxwidgets.org
>
More information about the wx-dev
mailing list