On Sun, Mar 24, 2013 at 12:16 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > This class implements the controller interface for Windows/mingw. > --- > SpiceXPI/src/plugin/Makefile.am | 2 + > SpiceXPI/src/plugin/controller-win.cpp | 266 +++++++++++++++++++++++++++++++++ > SpiceXPI/src/plugin/controller-win.h | 93 ++++++++++++ > SpiceXPI/src/plugin/controller.cpp | 15 ++ > SpiceXPI/src/plugin/plugin.cpp | 11 ++ > configure.ac | 6 +- > 6 files changed, 390 insertions(+), 3 deletions(-) > create mode 100644 SpiceXPI/src/plugin/controller-win.cpp > create mode 100644 SpiceXPI/src/plugin/controller-win.h > > diff --git a/SpiceXPI/src/plugin/Makefile.am b/SpiceXPI/src/plugin/Makefile.am > index bb50d21..93f4c2c 100644 > --- a/SpiceXPI/src/plugin/Makefile.am > +++ b/SpiceXPI/src/plugin/Makefile.am > @@ -66,6 +66,8 @@ if OS_WINDOWS > $(LIBTOOL) --tag=RC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(WINDRES) $(RCFLAGS) -i $< -o $@ > > npSpiceConsole_la_SOURCES += \ > + controller-win.cpp \ > + controller-win.h \ > resource.rc \ > $(NULL) > > diff --git a/SpiceXPI/src/plugin/controller-win.cpp b/SpiceXPI/src/plugin/controller-win.cpp > new file mode 100644 > index 0000000..7158624 > --- /dev/null > +++ b/SpiceXPI/src/plugin/controller-win.cpp > @@ -0,0 +1,266 @@ > +/* ***** BEGIN LICENSE BLOCK ***** > + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 > + * > + * The contents of this file are subject to the Mozilla Public License Version > + * 1.1 (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * http://www.mozilla.org/MPL/ > + * > + * Software distributed under the License is distributed on an "AS IS" basis, > + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License > + * for the specific language governing rights and limitations under the > + * License. > + * > + * Copyright 2009-2011, Red Hat Inc. > + * Copyright 2013, Red Hat Inc. 2009-2013 > + * Based on mozilla.org's scriptable plugin example > + * > + * The Original Code is mozilla.org code. > + * > + * The Initial Developer of the Original Code is > + * Netscape Communications Corporation. > + * Portions created by the Initial Developer are Copyright (C) 1998 > + * the Initial Developer. All Rights Reserved. > + * > + * Contributor(s): > + * Uri Lublin > + * Martin Stransky > + * Peter Hatina > + * Christophe Fergeau > + * > + * Alternatively, the contents of this file may be used under the terms of > + * either the GNU General Public License Version 2 or later (the "GPL"), or > + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), > + * in which case the provisions of the GPL or the LGPL are applicable instead > + * of those above. If you wish to allow use of your version of this file only > + * under the terms of either the GPL or the LGPL, and not to allow others to > + * use your version of this file under the terms of the MPL, indicate your > + * decision by deleting the provisions above and replace them with the notice > + * and other provisions required by the GPL or the LGPL. If you do not delete > + * the provisions above, a recipient may use your version of this file under > + * the terms of any one of the MPL, the GPL or the LGPL. > + * > + * ***** END LICENSE BLOCK ***** */ > + > +#include "config.h" > + > +#include <aclapi.h> > +#include <cstdio> > +#include <cstdlib> > +#include <cstring> > +#include <cerrno> > +#include <glib.h> > +#include <gio/gwin32outputstream.h> > + > +#include "rederrorcodes.h" > +#include "controller-win.h" > +#include "plugin.h" > + > +SpiceControllerWin::SpiceControllerWin(nsPluginInstance *aPlugin): > + SpiceController(aPlugin) > +{ > + g_random_set_seed(GPOINTER_TO_INT(aPlugin)); > +} > + Time is probably better than plugin pointer, it is likely that 2 instances share the same space (think 2 users conflicting for pipe names), unless they use ASLR (afaik, mingw-fedora doesn't currently compile with that for example) > +SpiceControllerWin::~SpiceControllerWin() > +{ > +} > + > +int SpiceControllerWin::Connect() > +{ > + HANDLE hClientPipe; > + > + hClientPipe = CreateFile(m_name.c_str(), > + GENERIC_READ | GENERIC_WRITE, > + 0, NULL, > + OPEN_EXISTING, > + SECURITY_SQOS_PRESENT | SECURITY_ANONYMOUS, > + NULL); I would simplify a little, just g_return_val_if_fail(hClientPipe != INVALID_HANDLE_VALUE, -1) here instead of extra conditions and branches below. > + if (hClientPipe != INVALID_HANDLE_VALUE) { > + g_warning("Connection OK"); > + m_pipe = g_win32_output_stream_new(hClientPipe, TRUE); > + return 0; > + } else { > + g_warning("Connection failed"); > + return -1; > + } > + g_return_val_if_reached(-1); > +} In general, returning true on success is easier to read. > +static int get_sid(HANDLE handle, PSID* ppsid, PSECURITY_DESCRIPTOR* ppsec_desc) > +{ > + DWORD ret = GetSecurityInfo(handle, SE_KERNEL_OBJECT, OWNER_SECURITY_INFORMATION, > + ppsid, NULL, NULL, NULL, ppsec_desc); > + if (ret != ERROR_SUCCESS) { > + return ret; > + } > + if (!IsValidSid(*ppsid)) { > + return -1; > + } > + return 0; > +} > + > +//checks whether the handle owner is the current user. > +static bool is_same_user(HANDLE handle) > +{ > + PSECURITY_DESCRIPTOR psec_desc_handle = NULL; > + PSECURITY_DESCRIPTOR psec_desc_user = NULL; > + PSID psid_handle; > + PSID psid_user; > + bool ret; > + > + ret = !get_sid(handle, &psid_handle, &psec_desc_handle) && > + !get_sid(GetCurrentProcess(), &psid_user, &psec_desc_user) && > + EqualSid(psid_handle, psid_user); That would make this easier to read. > + LocalFree(psec_desc_handle); > + LocalFree(psec_desc_user); > + return ret; > +} > + > +bool SpiceControllerWin::CheckPipe() > +{ > + void *hClientPipe; > + > + if (!G_IS_WIN32_OUTPUT_STREAM(m_pipe)) > + return false; g_return_val_if_fail(G_IS_WIN32_OUTPUT_STREAM(m_pipe), FALSE) is more idiomatic. > + hClientPipe = g_win32_output_stream_get_handle(G_WIN32_OUTPUT_STREAM(m_pipe)); > + // Verify the named-pipe-server owner is the current user. > + // Do it here so upon failure use next condition cleanup code. > + if (hClientPipe != INVALID_HANDLE_VALUE) { Can this happen? If not, it would be nicer to have g_return pre-condition for that. > + if (!is_same_user(hClientPipe)) { > + g_critical("Closing pipe to spicec -- it is not safe"); > + g_object_unref(G_OBJECT(m_pipe)); > + m_pipe = NULL; > + return false; > + } > + } > + > + return true; > +} > + > +#ifdef _UNICODE > +#define _T L > +#else > +#define _T > +#endif There is a standard windows TEXT macro doing this. > +#define SPICE_CLIENT_REGISTRY_KEY _T("Software\\spice-space.org\\spicex") > +#define SPICE_XPI_DLL _T("npSpiceConsole.dll") > +#define RED_CLIENT_FILE_NAME _T("spicec.exe") > +#define CMDLINE_LENGTH 32768 > + > +GStrv SpiceControllerWin::GetClientPath() > +{ > + LONG lret; > + HKEY hkey; > + > + lret = RegOpenKeyEx(HKEY_CURRENT_USER, SPICE_CLIENT_REGISTRY_KEY, > + 0, KEY_READ, &hkey); > + if (lret == ERROR_SUCCESS) { I would use g_return_val_if_fail(), that would give some clue on error, and avoid extra branches. > + DWORD dwType = REG_SZ; > + TCHAR lpCommandLine[CMDLINE_LENGTH] = {0}; > + DWORD dwSize = sizeof(lpCommandLine); > + GArray *argv_garray; > + char *it; > + > + lret = RegQueryValueEx(hkey, "client", NULL, &dwType, > + (LPBYTE)lpCommandLine, &dwSize); > + RegCloseKey(hkey); > + > + /* Some convoluted code here, the registry key contains the > + * command to run as a string, the GSpawn API expects an array > + * of strings. The awkward part is that the GSpawn API will > + * then rebuild a commandline string from this array :-/ */ > + argv_garray = g_array_new(TRUE, FALSE, sizeof(char *)); GPtrArray is more convenient for pointers, > + /* Look for .exe from the end as the path in which the SPICE > + * client is installed may contain '.exe' */ > + it = g_strrstr_len(lpCommandLine, dwSize, ".exe "); > + if (it != NULL) { > + gchar **args; > + gchar *val; > + it += strlen(".exe"); > + *it = '\0'; > + it++; > + val = g_strdup(lpCommandLine); > + g_array_prepend_val(argv_garray, val); > + args = g_strsplit(it, " ", -1); > + g_array_append_vals(argv_garray, args, g_strv_length(args)); > + /* We don't want to free the strings stored in args, just > + * the container */ > + g_free(args); > + return (GStrv)g_array_free(argv_garray, FALSE); I wonder if g_shell_parse_argv() could do that. > + } > + } > + > + g_warn_if_reached(); It's nicer to check the condition where it happens rather than having dead branches. > + return NULL; > +} > + > +GStrv SpiceControllerWin::GetFallbackClientPath() > +{ > + HMODULE hModule; > + // we assume the Spice client binary is located in the same dir as the > + // Firefox plugin > + if ((hModule = GetModuleHandle(SPICE_XPI_DLL))) { > + gchar *module_path; > + GStrv fallback_argv; > + > + module_path = g_win32_get_package_installation_directory_of_module(hModule); > + if (module_path != NULL) { > + fallback_argv = g_new0(char *, 3); > + fallback_argv[0] = g_build_filename(module_path, RED_CLIENT_FILE_NAME, NULL); > + fallback_argv[1] = g_strdup("--controller"); > + g_free(module_path); > + return fallback_argv; > + } > + } > + > + g_warn_if_reached(); Same remarks as above: I'd check each condition, avoid extra conditon branches and dead end. > + return NULL; > +} > + > +#define RED_CLIENT_PIPE_NAME TEXT("\\\\.\\pipe\\SpiceController-%lu") > +void SpiceControllerWin::SetupControllerPipe(GStrv &env) > +{ > + char *pipe_name; > + pipe_name = g_strdup_printf(RED_CLIENT_PIPE_NAME, (unsigned long)g_random_int()); > + this->SetFilename(pipe_name); > + /* FIXME set SPICE_XPI_NAMEDPIPE */ still fixme? > + env = g_environ_setenv(env, "SPICE_XPI_NAMEDPIPE", pipe_name, TRUE); > + g_free(pipe_name); > +} > + > +void SpiceControllerWin::StopClient() > +{ > + if (m_pid_controller != NULL) { > + //WaitForPid will take care of closing the handle > + TerminateProcess(m_pid_controller, 0); > + m_pid_controller = NULL; > + } > +} > + > + > +uint32_t SpiceControllerWin::Write(const void *lpBuffer, uint32_t nBytesToWrite) > +{ > + GError *error = NULL; > + gsize bytes_written; > + > + g_return_val_if_fail(G_IS_OUTPUT_STREAM(m_pipe), 0); > + > + g_output_stream_write_all(m_pipe, lpBuffer, nBytesToWrite, > + &bytes_written, NULL, &error); > + if (error != NULL) { > + g_warning("Error writing to controller pipe: %s", error->message); > + g_clear_error(&error); > + return -1; > + } > + if (bytes_written != nBytesToWrite) { > + g_warning("Partial write (%"G_GSIZE_MODIFIER"u instead of %u", > + bytes_written, (unsigned int)nBytesToWrite); > + } > + > + return bytes_written; > +} > diff --git a/SpiceXPI/src/plugin/controller-win.h b/SpiceXPI/src/plugin/controller-win.h > new file mode 100644 > index 0000000..fe09900 > --- /dev/null > +++ b/SpiceXPI/src/plugin/controller-win.h > @@ -0,0 +1,93 @@ > +/* ***** BEGIN LICENSE BLOCK ***** > + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 > + * > + * The contents of this file are subject to the Mozilla Public License Version > + * 1.1 (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * http://www.mozilla.org/MPL/ > + * > + * Software distributed under the License is distributed on an "AS IS" basis, > + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License > + * for the specific language governing rights and limitations under the > + * License. > + * > + * Copyright 2009-2011, Red Hat Inc. > + * Copyright 2013, Red Hat Inc. > + * Based on mozilla.org's scriptable plugin example > + * > + * The Original Code is mozilla.org code. > + * > + * The Initial Developer of the Original Code is > + * Netscape Communications Corporation. > + * Portions created by the Initial Developer are Copyright (C) 1998 > + * the Initial Developer. All Rights Reserved. > + * > + * Contributor(s): > + * Uri Lublin > + * Martin Stransky > + * Peter Hatina > + * Christophe Fergeau > + * > + * Alternatively, the contents of this file may be used under the terms of > + * either the GNU General Public License Version 2 or later (the "GPL"), or > + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), > + * in which case the provisions of the GPL or the LGPL are applicable instead > + * of those above. If you wish to allow use of your version of this file only > + * under the terms of either the GPL or the LGPL, and not to allow others to > + * use your version of this file under the terms of the MPL, indicate your > + * decision by deleting the provisions above and replace them with the notice > + * and other provisions required by the GPL or the LGPL. If you do not delete > + * the provisions above, a recipient may use your version of this file under > + * the terms of any one of the MPL, the GPL or the LGPL. > + * > + * ***** END LICENSE BLOCK ***** */ > + > +#ifndef SPICE_CONTROLLER_WIN_H > +#define SPICE_CONTROLLER_WIN_H > + > +/* > + Basic assumption: > + ------------------ > + Cross platform compatible. > + Easy to transform into remote process communication > + Secured > + > + chosen: > + Unix - Unix Domain Sockets (easy to change into regular sockets for remote communication) > + Windows - Named pipe (which allows remote access and is duplex > + (rather than anonymous pipe which is local only and one way) > +*/ > + > +#include <glib.h> > +#include <glib-object.h> /* for GStrv */ > +#include <gio/gio.h> > +#include <string> > +extern "C" { > +# include <stdint.h> > +# include <limits.h> > +} > + > +#include <spice/controller_prot.h> > +#include "controller.h" > + > +class nsPluginInstance; > + > +class SpiceControllerWin: public SpiceController > +{ > +public: > + SpiceControllerWin(nsPluginInstance *aPlugin); > + virtual ~SpiceControllerWin(); > + > + virtual void StopClient(); > + virtual uint32_t Write(const void *lpBuffer, uint32_t nBytesToWrite); > + int Connect(int nRetries) { return SpiceController::Connect(nRetries); }; > + > +private: > + virtual int Connect(); > + virtual void SetupControllerPipe(GStrv &env); > + virtual bool CheckPipe(); > + virtual GStrv GetClientPath(void); > + virtual GStrv GetFallbackClientPath(void); > +}; > + > +#endif // SPICE_CONTROLLER_WIN_H > diff --git a/SpiceXPI/src/plugin/controller.cpp b/SpiceXPI/src/plugin/controller.cpp > index ccef1d4..ac97ce1 100644 > --- a/SpiceXPI/src/plugin/controller.cpp > +++ b/SpiceXPI/src/plugin/controller.cpp > @@ -97,6 +97,21 @@ int SpiceController::Connect(const int nRetries) > g_usleep(sleep_time * G_USEC_PER_SEC); > ++sleep_time; > } > + if (rc != 0) { > + g_warning("error connecting"); > + g_assert(m_pipe == NULL); > + } > + if (!CheckPipe()) { > + g_warning("Pipe validation failure"); > + g_warn_if_fail(m_pipe == NULL); > + } > + if (m_pipe == NULL) { > + g_warning("failed to create pipe"); > +#ifdef XP_WIN > + rc = MAKE_HRESULT(1, FACILITY_CREATE_RED_PIPE, GetLastError()); > +#endif > + this->StopClient(); > + } > > return rc; > } > diff --git a/SpiceXPI/src/plugin/plugin.cpp b/SpiceXPI/src/plugin/plugin.cpp > index cd17620..2e59bfd 100644 > --- a/SpiceXPI/src/plugin/plugin.cpp > +++ b/SpiceXPI/src/plugin/plugin.cpp > @@ -66,7 +66,12 @@ extern "C" { > #include <fstream> > #include <set> > > +#if defined(XP_UNIX) > #include "controller-unix.h" > +#endif > +#if defined(XP_WIN) > +#include "controller-win.h" > +#endif > #include "plugin.h" > #include "nsScriptablePeer.h" > > @@ -187,7 +192,13 @@ nsPluginInstance::nsPluginInstance(NPP aInstance): > g_type_init(); > #endif > > +#if defined(XP_WIN) > + m_external_controller = new SpiceControllerWin(this); > +#elif defined(XP_UNIX) > m_external_controller = new SpiceControllerUnix(this); > +#else > +#error "Unknown OS, no controller implementation" > +#endif > } > > nsPluginInstance::~nsPluginInstance() > diff --git a/configure.ac b/configure.ac > index 7f401ab..39a1f7e 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -41,9 +41,9 @@ AC_CONFIG_SUBDIRS([spice-protocol]) > SPICE_PROTOCOL_CFLAGS='-I ${top_srcdir}/spice-protocol' > AC_SUBST(SPICE_PROTOCOL_CFLAGS) > > -PKG_CHECK_MODULES(GLIB, glib-2.0 gio-2.0 gthread-2.0) > -AC_SUBST(GLIB_CFLAGS) > -AC_SUBST(GLIB_LIBS) > +SPICE_XPI_REQUIRES="glib-2.0 gio-2.0 gthread-2.0" > +AS_IF([test "x$backend" = "xwindows"], [SPICE_XPI_REQUIRES="$SPICE_XPI_REQUIRES gio-windows-2.0"]) > +PKG_CHECK_MODULES([GLIB],[$SPICE_XPI_REQUIRES]) > > AC_ARG_ENABLE([xpi], > [AS_HELP_STRING([--enable-xpi], > -- > 1.8.1.4 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel