[Spice-devel] [spice-xpi 4/5] Add SpiceControllerWin class
Marc-André Lureau
marcandre.lureau at gmail.com
Sun Mar 24 15:27:45 PDT 2013
On Sun, Mar 24, 2013 at 12:16 PM, Christophe Fergeau
<cfergeau at redhat.com> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
More information about the Spice-devel
mailing list