Remove QueryConnectionHandler

Make things simpler by making this a part of the SDesktop interface
that always needs to be implemented.

common/rfb/SDesktop.h

@@ -44,6 +44,8 @@
 #include <rfb/screenTypes.h>
 #include <rfb/util.h>
 
+namespace network { class Socket; }
+
 namespace rfb {
 
   class VNCServer;
@@ -56,14 +58,22 @@ namespace rfb {
     // set via the VNCServer's setPixelBuffer() method by the time this call
     // returns.
 
-    virtual void start(VNCServer* __unused_attr vs) {}
+    virtual void start(VNCServer* vs) = 0;
 
     // stop() is called by the server when there are no longer any
     // authenticated clients, and therefore the desktop can cease any
     // expensive tasks.  No further calls to the VNCServer passed to start()
     // can be made once stop has returned.
 
-    virtual void stop() {}
+    virtual void stop() = 0;
+
+    // queryConnection() is called when a connection has been
+    // successfully authenticated.  The sock and userName arguments
+    // identify the socket and the name of the authenticated user, if
+    // any. At some point later VNCServer::approveConnection() should
+    // be called to either accept or reject the client.
+    virtual void queryConnection(network::Socket* sock,
+                                 const char* userName) = 0;
 
     // setScreenLayout() requests to reconfigure the framebuffer and/or
     // the layout of screens.
@@ -112,6 +122,10 @@ namespace rfb {
       server->setPixelBuffer(0);
       server = 0;
     }
+    virtual void queryConnection(network::Socket* sock,
+                                 const char* userName) {
+      server->approveConnection(sock, true, NULL);
+    }
 
   protected:
     VNCServer* server;

common/rfb/VNCSConnectionST.cxx

@@ -414,8 +414,6 @@ void VNCSConnectionST::authSuccess()
 {
   lastEventTime = time(0);
 
-  server->startDesktop();
-
   // - Set the connection parameters appropriately
   cp.width = server->pb->width();
   cp.height = server->pb->height();
@@ -440,6 +438,9 @@ void VNCSConnectionST::queryConnection(const char* userName)
   CharArray name; name.buf = sock->getPeerAddress();
   server->blHosts->clearBlackmark(name.buf);
 
+  // - Prepare the desktop that we might be making calls
+  server->startDesktop();
+
   // - Special case to provide a more useful error message
   if (rfb::Server::neverShared && !rfb::Server::disconnectClients &&
     server->authClientCount() > 0) {

common/rfb/VNCServer.h

@@ -26,6 +26,8 @@
 #include <rfb/SSecurity.h>
 #include <rfb/ScreenSet.h>
 
+namespace network { class Socket; }
+
 namespace rfb {
 
   class VNCServer : public UpdateTracker {
@@ -59,6 +61,14 @@ namespace rfb {
     // bell() tells the server that it should make all clients make a bell sound.
     virtual void bell() = 0;
 
+    // approveConnection() is called some time after
+    // SDesktop::queryConnection() has been called, to accept or reject
+    // the connection.  The accept argument should be true for
+    // acceptance, or false for rejection, in which case a string
+    // reason may also be given.
+    virtual void approveConnection(network::Socket* sock, bool accept,
+                                   const char* reason = NULL) = 0;
+
     // - Close all currently-connected clients, by calling
     //   their close() method with the supplied reason.
     virtual void closeClients(const char* reason) = 0;

common/rfb/VNCServerST.cxx

@@ -80,7 +80,7 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_)
     name(strDup(name_)), pointerClient(0), comparer(0),
     cursor(new Cursor(0, 0, Point(), NULL)),
     renderedCursorInvalid(false),
-    queryConnectionHandler(0), keyRemapper(&KeyRemapper::defInstance),
+    keyRemapper(&KeyRemapper::defInstance),
     lastConnectionTime(0), disableclients(false),
     frameTimer(this)
 {
@@ -534,6 +534,12 @@ bool VNCServerST::handleTimeout(Timer* t)
   return false;
 }
 
+void VNCServerST::queryConnection(network::Socket* sock,
+                                  const char* userName)
+{
+  desktop->queryConnection(sock, userName);
+}
+
 // -=- Internal methods
 
 void VNCServerST::startDesktop()

common/rfb/VNCServerST.h

@@ -95,6 +95,11 @@ namespace rfb {
     virtual void setScreenLayout(const ScreenSet& layout);
     virtual PixelBuffer* getPixelBuffer() const { return pb; }
     virtual void serverCutText(const char* str, int len);
+
+    virtual void approveConnection(network::Socket* sock, bool accept,
+                                   const char* reason);
+    virtual void closeClients(const char* reason) {closeClients(reason, 0);}
+
     virtual void add_changed(const Region &region);
     virtual void add_copied(const Region &dest, const Point &delta);
     virtual void setCursor(int width, int height, const Point& hotspot,
@@ -104,10 +109,6 @@ namespace rfb {
 
     virtual void bell();
 
-    // - Close all currently-connected clients, by calling
-    //   their close() method with the supplied reason.
-    virtual void closeClients(const char* reason) {closeClients(reason, 0);}
-
     // VNCServerST-only methods
 
     // closeClients() closes all RFB sessions, except the specified one (if
@@ -128,42 +129,13 @@ namespace rfb {
     // clients
     virtual void setName(const char* name_);
 
-    // A QueryConnectionHandler, if supplied, is passed details of incoming
-    // connections to approve, reject, or query the user about.
-    //
     // queryConnection() is called when a connection has been
     // successfully authenticated.  The sock and userName arguments identify
     // the socket and the name of the authenticated user, if any.
     // approveConnection() must be called some time later to accept or reject
     // the connection.
-    struct QueryConnectionHandler {
-      virtual ~QueryConnectionHandler() {}
-      virtual void queryConnection(network::Socket* sock,
-                                   const char* userName) = 0;
-    };
-    void setQueryConnectionHandler(QueryConnectionHandler* qch) {
-      queryConnectionHandler = qch;
-    }
-
-    // queryConnection is called as described above, and either passes the
-    // request on to the registered handler, or accepts the connection if
-    // no handler has been specified.
     virtual void queryConnection(network::Socket* sock,
-                                 const char* userName) {
-      if (queryConnectionHandler) {
-        queryConnectionHandler->queryConnection(sock, userName);
-        return;
-      }
-      approveConnection(sock, true, NULL);
-    }
-
-    // approveConnection() is called by the active QueryConnectionHandler,
-    // some time after queryConnection() has returned with PENDING, to accept
-    // or reject the connection.  The accept argument should be true for
-    // acceptance, or false for rejection, in which case a string reason may
-    // also be given.
-    void approveConnection(network::Socket* sock, bool accept,
-                           const char* reason);
+                                 const char* userName);
 
     // setBlacklist() is called to replace the VNCServerST's internal
     // Blacklist instance with another instance.  This allows a single
@@ -231,7 +203,6 @@ namespace rfb {
 
     bool getComparerState();
 
-    QueryConnectionHandler* queryConnectionHandler;
     KeyRemapper* keyRemapper;
 
     time_t lastUserInputTime;

unix/x0vncserver/XDesktop.cxx

@@ -18,6 +18,8 @@
  * USA.
  */
 
+#include <assert.h>
+
 #include <x0vncserver/XDesktop.h>
 
 #include <X11/XKBlib.h>
@@ -53,6 +55,10 @@ BoolParameter rawKeyboard("RawKeyboard",
                           "Send keyboard events straight through and "
                           "avoid mapping them to the current keyboard "
                           "layout", false);
+IntParameter queryConnectTimeout("QueryConnectTimeout",
+                                 "Number of seconds to show the Accept Connection dialog before "
+                                 "rejecting the connection",
+                                 10);
 
 static rfb::LogWriter vlog("XDesktop");
 
@@ -63,6 +69,7 @@ static const char * ledNames[XDESKTOP_N_LEDS] = {
 
 XDesktop::XDesktop(Display* dpy_, Geometry *geometry_)
   : dpy(dpy_), geometry(geometry_), pb(0), server(0),
+    queryConnectDialog(0), queryConnectSock(0),
     oldButtonMask(0), haveXtest(false), haveDamage(false),
     maxButtons(0), running(false), ledMasks(), ledState(0),
     codeMap(0), codeMapLen(0)
@@ -254,6 +261,9 @@ void XDesktop::stop() {
     XDamageDestroy(dpy, damage);
 #endif
 
+  delete queryConnectDialog;
+  queryConnectDialog = 0;
+
   server->setPixelBuffer(0);
   server = 0;
 
@@ -265,6 +275,30 @@ bool XDesktop::isRunning() {
   return running;
 }
 
+void XDesktop::queryConnection(network::Socket* sock,
+                               const char* userName)
+{
+  assert(isRunning());
+
+  if (queryConnectSock) {
+    server->approveConnection(sock, false, "Another connection is currently being queried.");
+    return;
+  }
+
+  if (!userName)
+    userName = "(anonymous)";
+
+  queryConnectSock = sock;
+
+  CharArray address(sock->getPeerAddress());
+  delete queryConnectDialog;
+  queryConnectDialog = new QueryConnectDialog(dpy, address.buf,
+                                              userName,
+                                              queryConnectTimeout,
+                                              this);
+  queryConnectDialog->map();
+}
+
 void XDesktop::pointerEvent(const Point& pos, int buttonMask) {
 #ifdef HAVE_XTEST
   if (!haveXtest) return;
@@ -684,6 +718,21 @@ bool XDesktop::handleGlobalEvent(XEvent* ev) {
   return false;
 }
 
+void XDesktop::queryApproved()
+{
+  assert(isRunning());
+  server->approveConnection(queryConnectSock, true, 0);
+  queryConnectSock = 0;
+}
+
+void XDesktop::queryRejected()
+{
+  assert(isRunning());
+  server->approveConnection(queryConnectSock, false,
+                            "Connection rejected by local user");
+  queryConnectSock = 0;
+}
+
 bool XDesktop::setCursor()
 {
   XFixesCursorImage *cim;

unix/x0vncserver/XDesktop.h

@@ -30,13 +30,17 @@
 #include <X11/extensions/Xdamage.h>
 #endif
 
+#include <vncconfig/QueryConnectDialog.h>
+
 class Geometry;
 class XPixelBuffer;
 
 // number of XKb indicator leds to handle
 #define XDESKTOP_N_LEDS 3
 
-class XDesktop : public rfb::SDesktop, public TXGlobalEventHandler
+class XDesktop : public rfb::SDesktop,
+                 public TXGlobalEventHandler,
+                 public QueryResultCallback
 {
 public:
   XDesktop(Display* dpy_, Geometry *geometry);
@@ -46,6 +50,8 @@ public:
   virtual void start(rfb::VNCServer* vs);
   virtual void stop();
   bool isRunning();
+  virtual void queryConnection(network::Socket* sock,
+                               const char* userName);
   virtual void pointerEvent(const rfb::Point& pos, int buttonMask);
   KeyCode XkbKeysymToKeycode(Display* dpy, KeySym keysym);
   virtual void keyEvent(rdr::U32 keysym, rdr::U32 xtcode, bool down);
@@ -56,11 +62,17 @@ public:
   // -=- TXGlobalEventHandler interface
   virtual bool handleGlobalEvent(XEvent* ev);
 
+  // -=- QueryResultCallback interface
+  virtual void queryApproved();
+  virtual void queryRejected();
+
 protected:
   Display* dpy;
   Geometry* geometry;
   XPixelBuffer* pb;
   rfb::VNCServerST* server;
+  QueryConnectDialog* queryConnectDialog;
+  network::Socket* queryConnectSock;
   int oldButtonMask;
   bool haveXtest;
   bool haveDamage;

unix/x0vncserver/x0vncserver.cxx

@@ -33,8 +33,6 @@
 #include <network/TcpSocket.h>
 #include <network/UnixSocket.h>
 
-#include <vncconfig/QueryConnectDialog.h>
-
 #include <signal.h>
 #include <X11/X.h>
 #include <X11/Xlib.h>
@@ -61,10 +59,6 @@ StringParameter displayname("display", "The X display", "");
 IntParameter rfbport("rfbport", "TCP port to listen for RFB protocol",5900);
 StringParameter rfbunixpath("rfbunixpath", "Unix socket to listen for RFB protocol", "");
 IntParameter rfbunixmode("rfbunixmode", "Unix socket access mode", 0600);
-IntParameter queryConnectTimeout("QueryConnectTimeout",
-                                 "Number of seconds to show the Accept Connection dialog before "
-                                 "rejecting the connection",
-                                 10);
 StringParameter hostsFile("HostsFile", "File with IP access control rules", "");
 
 //
@@ -79,48 +73,6 @@ static void CleanupSignalHandler(int sig)
 }
 
 
-class QueryConnHandler : public VNCServerST::QueryConnectionHandler,
-                         public QueryResultCallback {
-public:
-  QueryConnHandler(Display* dpy, VNCServerST* vs)
-    : display(dpy), server(vs), queryConnectDialog(0), queryConnectSock(0) {}
-  ~QueryConnHandler() { delete queryConnectDialog; }
-
-  // -=- VNCServerST::QueryConnectionHandler interface
-  virtual void queryConnection(network::Socket* sock,
-                               const char* userName) {
-    if (queryConnectSock) {
-      server->approveConnection(sock, false, "Another connection is currently being queried.");
-      return;
-    }
-    if (!userName) userName = "(anonymous)";
-    queryConnectSock = sock;
-    CharArray address(sock->getPeerAddress());
-    delete queryConnectDialog;
-    queryConnectDialog = new QueryConnectDialog(display, address.buf,
-                                                userName, queryConnectTimeout,
-                                                this);
-    queryConnectDialog->map();
-  }
-
-  // -=- QueryResultCallback interface
-  virtual void queryApproved() {
-    server->approveConnection(queryConnectSock, true, 0);
-    queryConnectSock = 0;
-  }
-  virtual void queryRejected() {
-    server->approveConnection(queryConnectSock, false,
-                              "Connection rejected by local user");
-    queryConnectSock = 0;
-  }
-private:
-  Display* display;
-  VNCServerST* server;
-  QueryConnectDialog* queryConnectDialog;
-  network::Socket* queryConnectSock;
-};
-
-
 class FileTcpFilter : public TcpFilter
 {
 
@@ -305,8 +257,6 @@ int main(int argc, char** argv)
     XDesktop desktop(dpy, &geo);
 
     VNCServerST server("x0vncserver", &desktop);
-    QueryConnHandler qcHandler(dpy, &server);
-    server.setQueryConnectionHandler(&qcHandler);
 
     if (rfbunixpath.getValueStr()[0] != '\0') {
       listeners.push_back(new network::UnixListener(rfbunixpath, rfbunixmode));

unix/xserver/hw/vnc/XserverDesktop.cc

@@ -81,7 +81,6 @@ XserverDesktop::XserverDesktop(int screenIndex_,
 
   server = new VNCServerST(name, this);
   setFramebuffer(width, height, fbptr, stride);
-  server->setQueryConnectionHandler(this);
 
   for (std::list<SocketListener*>::iterator i = listeners.begin();
        i != listeners.end();
@@ -145,6 +144,17 @@ void XserverDesktop::refreshScreenLayout()
   server->setScreenLayout(::computeScreenLayout(&outputIdMap));
 }
 
+void XserverDesktop::start(rfb::VNCServer* vs)
+{
+  // We already own the server object, and we always keep it in a
+  // ready state
+  assert(vs == server);
+}
+
+void XserverDesktop::stop()
+{
+}
+
 void XserverDesktop::queryConnection(network::Socket* sock,
                                      const char* userName)
 {

unix/xserver/hw/vnc/XserverDesktop.h

@@ -45,7 +45,6 @@ namespace rfb {
 namespace network { class SocketListener; class Socket; class SocketServer; }
 
 class XserverDesktop : public rfb::SDesktop, public rfb::FullFramePixelBuffer,
-                       public rfb::VNCServerST::QueryConnectionHandler,
                        public rfb::Timer::Callback {
 public:
 
@@ -86,6 +85,10 @@ public:
                          const char* rejectMsg=0);
 
   // rfb::SDesktop callbacks
+  virtual void start(rfb::VNCServer* vs);
+  virtual void stop();
+  virtual void queryConnection(network::Socket* sock,
+                               const char* userName);
   virtual void pointerEvent(const rfb::Point& pos, int buttonMask);
   virtual void keyEvent(rdr::U32 keysym, rdr::U32 keycode, bool down);
   virtual void clientCutText(const char* str, int len);
@@ -95,10 +98,6 @@ public:
   // rfb::PixelBuffer callbacks
   virtual void grabRegion(const rfb::Region& r);
 
-  // rfb::VNCServerST::QueryConnectionHandler callback
-  virtual void queryConnection(network::Socket* sock,
-                               const char* userName);
-
 protected:
   bool handleListenerEvent(int fd,
                            std::list<network::SocketListener*>* sockets,

win/rfb_win32/SDisplay.cxx

@@ -20,6 +20,8 @@
 //
 // The SDisplay class encapsulates a particular system display.
 
+#include <assert.h>
+
 #include <rfb_win32/SDisplay.h>
 #include <rfb_win32/Service.h>
 #include <rfb_win32/TsSessions.h>
@@ -66,7 +68,7 @@ SDisplay::SDisplay()
   : server(0), pb(0), device(0),
     core(0), ptr(0), kbd(0), clipboard(0),
     inputs(0), monitor(0), cleanDesktop(0), cursor(0),
-    statusLocation(0), ledState(0)
+    statusLocation(0), queryConnectionHandler(0), ledState(0)
 {
   updateEvent.h = CreateEvent(0, TRUE, FALSE, 0);
 }
@@ -139,6 +141,20 @@ void SDisplay::stop()
 }
 
 
+void SDisplay::queryConnection(network::Socket* sock,
+                               const char* userName)
+{
+  assert(server != NULL);
+
+  if (queryConnectionHandler) {
+    queryConnectionHandler->queryConnection(sock, userName);
+    return;
+  }
+
+  server->approveConnection(sock, true);
+}
+
+
 void SDisplay::startCore() {
 
   // Currently, we just check whether we're in the console session, and

win/rfb_win32/SDisplay.h

@@ -52,6 +52,13 @@ namespace rfb {
       virtual const char* methodName() const = 0;
     };
 
+    class QueryConnectionHandler {
+    public:
+      virtual ~QueryConnectionHandler() {}
+      virtual void queryConnection(network::Socket* sock,
+                                   const char* userName) = 0;
+    };
+
     class SDisplay : public SDesktop,
       WMMonitor::Notifier,
       Clipboard::Notifier,
@@ -65,6 +72,8 @@ namespace rfb {
 
       virtual void start(VNCServer* vs);
       virtual void stop();
+      virtual void queryConnection(network::Socket* sock,
+                                   const char* userName);
       virtual void pointerEvent(const Point& pos, int buttonmask);
       virtual void keyEvent(rdr::U32 keysym, rdr::U32 keycode, bool down);
       virtual void clientCutText(const char* str, int len);
@@ -86,6 +95,12 @@ namespace rfb {
 
       void setStatusLocation(bool* status) {statusLocation = status;}
 
+      // -=- Set handler for incoming connections
+
+      void setQueryConnectionHandler(QueryConnectionHandler* qch) {
+        queryConnectionHandler = qch;
+      }
+
       static IntParameter updateMethod;
       static BoolParameter disableLocalInputs;
       static StringParameter disconnectAction;
@@ -150,6 +165,9 @@ namespace rfb {
       // -=- Where to write the active/inactive indicator to
       bool* statusLocation;
 
+      // -=- Whom to query incoming connections
+      QueryConnectionHandler* queryConnectionHandler;
+
       unsigned ledState;
     };
 

win/winvnc/VNCServerWin32.cxx

@@ -71,9 +71,7 @@ VNCServerWin32::VNCServerWin32()
 
   // Initialise the desktop
   desktop.setStatusLocation(&isDesktopStarted);
-
-  // Initialise the VNC server
-  vncServer.setQueryConnectionHandler(this);
+  desktop.setQueryConnectionHandler(this);
 
   // Register the desktop's event to be handled
   sockMgr.addEvent(desktop.getUpdateEvent(), &desktop);

win/winvnc/VNCServerWin32.h

@@ -39,7 +39,7 @@ namespace winvnc {
 
   class STrayIconThread;
 
-  class VNCServerWin32 : rfb::VNCServerST::QueryConnectionHandler,
+  class VNCServerWin32 : rfb::win32::QueryConnectionHandler,
                          rfb::win32::SocketManager::AddressChangeNotifier,
                          rfb::win32::RegConfig::Callback,
                          rfb::win32::EventHandler {
@@ -78,7 +78,7 @@ namespace winvnc {
     bool setClientsStatus(rfb::ListConnInfo* LCInfo);
 
   protected:
-    // VNCServerST::QueryConnectionHandler interface
+    // QueryConnectionHandler interface
     // Callback used to prompt user to accept or reject a connection.
     // CALLBACK IN VNCServerST "HOST" THREAD
     virtual void queryConnection(network::Socket* sock,