diff --git a/common/rfb/SDesktop.h b/common/rfb/SDesktop.h index 50c92c099f..8e3388377d 100644 --- a/common/rfb/SDesktop.h +++ b/common/rfb/SDesktop.h @@ -89,11 +89,9 @@ namespace rfb { // setScreenLayout() requests to reconfigure the framebuffer and/or // the layout of screens. - virtual unsigned int setScreenLayout(int /*fb_width*/, - int /*fb_height*/, - const ScreenSet& /*layout*/) { - return resultProhibited; - } + virtual void setScreenLayout(int /*fb_width*/, + int /*fb_height*/, + const ScreenSet& /*layout*/) = 0; // frameTick() is called whenever a frame update has been processed, // signalling that a good time to render new data diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx index d2bea8032a..8a21a19b26 100644 --- a/common/rfb/VNCSConnectionST.cxx +++ b/common/rfb/VNCSConnectionST.cxx @@ -376,6 +376,16 @@ void VNCSConnectionST::desktopReadyOrClose() } } +void VNCSConnectionST::setDesktopSizeDoneOrClose(uint16_t result) +{ + try { + if (state() != RFBSTATE_NORMAL) return; + setDesktopSizeDone(result); + } catch(std::exception& e) { + close(e.what()); + } +} + bool VNCSConnectionST::getComparerState() { // We interpret a low compression level as an indication that the client @@ -461,6 +471,10 @@ void VNCSConnectionST::desktopReady() SConnection::desktopReady(); } +void VNCSConnectionST::setDesktopSizeDone(uint16_t result) +{ + writer()->writeDesktopSize(reasonClient, result); +} void VNCSConnectionST::approveConnectionOrClose(bool accept, const char* reason) @@ -699,7 +713,6 @@ void VNCSConnectionST::framebufferUpdateRequest(const core::Rect& r, void VNCSConnectionST::setDesktopSize(int fb_width, int fb_height, const ScreenSet& layout) { - unsigned int result; char buffer[2048]; vlog.debug("Got request for framebuffer resize to %dx%d", @@ -709,12 +722,10 @@ void VNCSConnectionST::setDesktopSize(int fb_width, int fb_height, if (!accessCheck(AccessSetDesktopSize)) { vlog.debug("Rejecting unauthorized framebuffer resize request"); - result = resultProhibited; + setDesktopSizeDoneOrClose(resultProhibited); } else { - result = server->setDesktopSize(this, fb_width, fb_height, layout); + server->setDesktopSizeRequest(this, fb_width, fb_height, layout); } - - writer()->writeDesktopSize(reasonClient, result); } void VNCSConnectionST::fence(uint32_t flags, unsigned len, const uint8_t data[]) diff --git a/common/rfb/VNCSConnectionST.h b/common/rfb/VNCSConnectionST.h index 5e91e2cdc6..9913924608 100644 --- a/common/rfb/VNCSConnectionST.h +++ b/common/rfb/VNCSConnectionST.h @@ -85,6 +85,7 @@ namespace rfb { void announceClipboardOrClose(bool available); void sendClipboardDataOrClose(const char* data); void desktopReadyOrClose(); + void setDesktopSizeDoneOrClose(uint16_t result); // The following methods never throw exceptions @@ -172,6 +173,7 @@ namespace rfb { void setDesktopName(const char *name); void setLEDState(unsigned int state); void desktopReady() override; + void setDesktopSizeDone(uint16_t result); private: network::Socket* sock; diff --git a/common/rfb/VNCServer.h b/common/rfb/VNCServer.h index 0456aa3a35..16de82d348 100644 --- a/common/rfb/VNCServer.h +++ b/common/rfb/VNCServer.h @@ -95,6 +95,9 @@ namespace rfb { // the pixelbuffer. Clients will be notified of the new layout. virtual void setScreenLayout(const ScreenSet& layout) = 0; + // FIXME: Change code comments above + virtual void setScreenLayoutDone(uint16_t result) = 0; + // getPixelBuffer() returns a pointer to the PixelBuffer object. virtual const PixelBuffer* getPixelBuffer() const = 0; diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index 232477917a..42f4d590ae 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -88,12 +88,12 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_) : desktop(desktop_), desktopStarted(false), desktopStarting(false), blockCounter(0), pb(nullptr), ledState(ledUnknown), name(name_), pointerClient(nullptr), - clipboardClient(nullptr), pointerClientTime(0), - comparer(nullptr), cursor(new Cursor(0, 0, {}, nullptr)), - renderedCursorInvalid(false), + clipboardClient(nullptr), resizeClient(nullptr), + pointerClientTime(0), comparer(nullptr), + cursor(new Cursor(0, 0, {}, nullptr)), renderedCursorInvalid(false), keyRemapper(&KeyRemapper::defInstance), idleTimer(this), disconnectTimer(this), connectTimer(this), - msc(0), queuedMsc(0), frameTimer(this) + resizeTimer(this), msc(0), queuedMsc(0), frameTimer(this) { slog.debug("Creating single-threaded server %s", name.c_str()); @@ -607,50 +607,79 @@ void VNCServerST::handleClipboardData(VNCSConnectionST* client, desktop->handleClipboardData(data); } -unsigned int VNCServerST::setDesktopSize(VNCSConnectionST* requester, - int fb_width, int fb_height, - const ScreenSet& layout) +void VNCServerST::setDesktopSizeRequest(VNCSConnectionST* requester, + int fb_width, int fb_height, + const ScreenSet& layout) { - unsigned int result; - std::list::iterator ci; + + if (resizeClient) { + slog.debug("Rejecting concurrent framebuffer resize request"); + requester->setDesktopSizeDoneOrClose(resultNoResources); + return; + } if (!rfb::Server::acceptSetDesktopSize) { slog.debug("Rejecting unauthorized framebuffer resize request"); - return resultProhibited; + requester->setDesktopSizeDoneOrClose(resultNoResources); + return; } // We can't handle a framebuffer larger than this, so don't let a // client set one (see PixelBuffer.cxx) if ((fb_width > 16384) || (fb_height > 16384)) { slog.error("Rejecting too large framebuffer resize request"); - return resultProhibited; + requester->setDesktopSizeDoneOrClose(resultNoResources); + return; } // Don't bother the desktop with an invalid configuration if (!layout.validate(fb_width, fb_height)) { slog.error("Invalid screen layout requested by client"); - return resultInvalid; + requester->setDesktopSizeDoneOrClose(resultNoResources); + return; } // FIXME: the desktop will call back to VNCServerST and an extra set // of ExtendedDesktopSize messages will be sent. This is okay // protocol-wise, but unnecessary. - result = desktop->setScreenLayout(fb_width, fb_height, layout); - if (result != resultSuccess) - return result; + resizeClient = requester; + resizeTimer.start(2000); + desktop->setScreenLayout(fb_width, fb_height, layout); +} + +void VNCServerST::setScreenLayoutDone(uint16_t result) +{ + std::list::iterator ci; + + // This means we've timed out + // FIXME: This doesn't feel robust... + if (!resizeClient) + return; + + if (result != resultSuccess) { + resizeClient->setDesktopSizeDoneOrClose(result); + resizeClient = nullptr; + resizeTimer.stop(); + return; + } // Sanity check - if (screenLayout != layout) + if (screenLayout != getScreenLayout()) { + resizeClient = nullptr; + resizeTimer.stop(); throw std::runtime_error("Desktop configured a different screen layout than requested"); + } // Notify other clients for (ci = clients.begin(); ci != clients.end(); ++ci) { - if ((*ci) == requester) + if ((*ci) == resizeClient) continue; (*ci)->screenLayoutChangeOrClose(reasonOtherClient); } - return resultSuccess; + resizeClient->setDesktopSizeDoneOrClose(result); + resizeClient = nullptr; + resizeTimer.stop(); } // Other public methods @@ -732,6 +761,11 @@ void VNCServerST::handleTimeout(core::Timer* t) } else if (t == &connectTimer) { slog.info("MaxConnectionTime reached, exiting"); desktop->terminate(); + } else if (t == &resizeTimer) { + assert(resizeClient); + // FIXME: This is not robust. + resizeClient->setDesktopSizeDoneOrClose(resultNoResources); + resizeClient = nullptr; } } diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h index 07392794fe..f1f28bbf19 100644 --- a/common/rfb/VNCServerST.h +++ b/common/rfb/VNCServerST.h @@ -85,6 +85,7 @@ namespace rfb { void setPixelBuffer(PixelBuffer* pb, const ScreenSet& layout) override; void setPixelBuffer(PixelBuffer* pb) override; void setScreenLayout(const ScreenSet& layout) override; + void setScreenLayoutDone(uint16_t result) override; const PixelBuffer* getPixelBuffer() const override { return pb; } void requestClipboard() override; @@ -127,9 +128,9 @@ namespace rfb { void handleClipboardAnnounce(VNCSConnectionST* client, bool available); void handleClipboardData(VNCSConnectionST* client, const char* data); - unsigned int setDesktopSize(VNCSConnectionST* requester, - int fb_width, int fb_height, - const ScreenSet& layout); + void setDesktopSizeRequest(VNCSConnectionST* requester, + int fb_width, int fb_height, + const ScreenSet& layout); // closeClients() closes all RFB sessions, except the specified one (if // any), and logs the specified reason for closure. @@ -194,6 +195,7 @@ namespace rfb { std::list clients; VNCSConnectionST* pointerClient; VNCSConnectionST* clipboardClient; + VNCSConnectionST* resizeClient; std::list clipboardRequestors; time_t pointerClientTime; @@ -210,6 +212,7 @@ namespace rfb { core::Timer idleTimer; core::Timer disconnectTimer; core::Timer connectTimer; + core::Timer resizeTimer; uint64_t msc, queuedMsc; core::Timer frameTimer; diff --git a/unix/w0vncserver/portals/PortalDesktop.cxx b/unix/w0vncserver/portals/PortalDesktop.cxx index c52cf1c936..a421f17833 100644 --- a/unix/w0vncserver/portals/PortalDesktop.cxx +++ b/unix/w0vncserver/portals/PortalDesktop.cxx @@ -158,11 +158,11 @@ void PortalDesktop::terminate() kill(getpid(), SIGTERM); } -unsigned int PortalDesktop::setScreenLayout(int /* fb_width */, - int /* fb_height */, - const rfb::ScreenSet& /* layout */) +void PortalDesktop::setScreenLayout(int /* fb_width */, + int /* fb_height */, + const rfb::ScreenSet& /* layout */) { - return rfb::resultProhibited; + server->setScreenLayoutDone(rfb::resultProhibited); } void PortalDesktop::keyEvent(uint32_t keysym, uint32_t keycode, bool down) diff --git a/unix/w0vncserver/portals/PortalDesktop.h b/unix/w0vncserver/portals/PortalDesktop.h index bc524600f7..ecf9b45885 100644 --- a/unix/w0vncserver/portals/PortalDesktop.h +++ b/unix/w0vncserver/portals/PortalDesktop.h @@ -45,8 +45,8 @@ class PortalDesktop : public rfb::SDesktop void queryConnection(network::Socket* sock, const char* userName) override; void terminate() override; - unsigned int setScreenLayout(int fb_width, int fb_height, - const rfb::ScreenSet& layout) override; + void setScreenLayout(int fb_width, int fb_height, + const rfb::ScreenSet& layout) override; void keyEvent(uint32_t keysym, uint32_t keycode, bool down) override; void pointerEvent(const core::Point& pos, uint16_t buttonMask) override; diff --git a/unix/w0vncserver/wayland/WaylandDesktop.cxx b/unix/w0vncserver/wayland/WaylandDesktop.cxx index e4dae37ff0..b06e7b477e 100644 --- a/unix/w0vncserver/wayland/WaylandDesktop.cxx +++ b/unix/w0vncserver/wayland/WaylandDesktop.cxx @@ -188,6 +188,12 @@ void WaylandDesktop::terminate() kill(getpid(), SIGTERM); } +void WaylandDesktop::setScreenLayout(int /*fb_width*/, int /*fb_height*/, + const rfb::ScreenSet& /*layout*/) +{ + server->setScreenLayoutDone(rfb::resultProhibited); +} + void WaylandDesktop::handleClipboardRequest() { if (!dataControl) diff --git a/unix/w0vncserver/wayland/WaylandDesktop.h b/unix/w0vncserver/wayland/WaylandDesktop.h index b12b4658ff..72dee855df 100644 --- a/unix/w0vncserver/wayland/WaylandDesktop.h +++ b/unix/w0vncserver/wayland/WaylandDesktop.h @@ -24,7 +24,7 @@ #include -namespace rfb { class VNCServer; } +namespace rfb { class VNCServer; struct ScreenSet; } namespace wayland { class Output; @@ -55,6 +55,9 @@ class WaylandDesktop : public rfb::SDesktop { const char* userName) override; void terminate() override; + void setScreenLayout(int fb_width, int fb_height, + const rfb::ScreenSet& layout) override; + virtual void handleClipboardRequest() override; virtual void handleClipboardAnnounce(bool available) override; virtual void handleClipboardData(const char* data) override; diff --git a/unix/x0vncserver/XDesktop.cxx b/unix/x0vncserver/XDesktop.cxx index b52b20e199..6f9037981d 100644 --- a/unix/x0vncserver/XDesktop.cxx +++ b/unix/x0vncserver/XDesktop.cxx @@ -687,14 +687,15 @@ static void GetSmallerMode(XRRScreenResources *res, } #endif /* HAVE_XRANDR */ -unsigned int XDesktop::setScreenLayout(int fb_width, int fb_height, - const rfb::ScreenSet& layout) +void XDesktop::setScreenLayout(int fb_width, int fb_height, + const rfb::ScreenSet& layout) { #ifdef HAVE_XRANDR XRRScreenResources *res = XRRGetScreenResources(dpy, DefaultRootWindow(dpy)); if (!res) { vlog.error("XRRGetScreenResources failed"); - return rfb::resultProhibited; + server->setScreenLayoutDone(rfb::resultProhibited); + return; } vncSetGlueContext(dpy, res); @@ -741,26 +742,30 @@ unsigned int XDesktop::setScreenLayout(int fb_width, int fb_height, if (outputId == None) { vlog.debug("Resize adjust: Could not find corresponding screen"); XRRFreeScreenResources(res); - return rfb::resultInvalid; + server->setScreenLayoutDone(rfb::resultInvalid); + return; } XRROutputInfo *output = XRRGetOutputInfo(dpy, res, outputId); if (!output) { vlog.debug("Resize adjust: XRRGetOutputInfo failed"); XRRFreeScreenResources(res); - return rfb::resultInvalid; + server->setScreenLayoutDone(rfb::resultInvalid); + return; } if (!output->crtc) { vlog.debug("Resize adjust: Selected output has no CRTC"); XRRFreeScreenResources(res); XRRFreeOutputInfo(output); - return rfb::resultInvalid; + server->setScreenLayoutDone(rfb::resultInvalid); + return; } XRRCrtcInfo *crtc = XRRGetCrtcInfo(dpy, res, output->crtc); if (!crtc) { vlog.debug("Resize adjust: XRRGetCrtcInfo failed"); XRRFreeScreenResources(res); XRRFreeOutputInfo(output); - return rfb::resultInvalid; + server->setScreenLayoutDone(rfb::resultInvalid); + return; } unsigned int swidth = iter->dimensions.width(); @@ -797,7 +802,8 @@ unsigned int XDesktop::setScreenLayout(int fb_width, int fb_height, } else { vlog.error("Failed to find smaller or equal screen size"); XRRFreeScreenResources(res); - return rfb::resultInvalid; + server->setScreenLayoutDone(rfb::resultInvalid); + return; } } @@ -827,20 +833,22 @@ unsigned int XDesktop::setScreenLayout(int fb_width, int fb_height, VNCSConnectionST::setDesktopSize. Another ExtendedDesktopSize with reason=0 will be sent in response to the changes seen by the event handler. */ - if (adjustedLayout != layout) - return rfb::resultInvalid; + if (adjustedLayout != layout) { + server->setScreenLayoutDone(rfb::resultInvalid); + return; + } // Explicitly update the server state with the result as there // can be corner cases where we don't get feedback from the X server server->setScreenLayout(computeScreenLayout()); - return ret; + server->setScreenLayoutDone(ret); #else (void)fb_width; (void)fb_height; (void)layout; - return rfb::resultProhibited; + server->setScreenLayoutDone(rfb::resultProhibited); #endif /* HAVE_XRANDR */ } diff --git a/unix/x0vncserver/XDesktop.h b/unix/x0vncserver/XDesktop.h index 89e2531dbb..7e73e87003 100644 --- a/unix/x0vncserver/XDesktop.h +++ b/unix/x0vncserver/XDesktop.h @@ -66,8 +66,8 @@ class XDesktop : public rfb::SDesktop, void pointerEvent(const core::Point& pos, uint16_t buttonMask) override; void keyEvent(uint32_t keysym, uint32_t xtcode, bool down) override; - unsigned int setScreenLayout(int fb_width, int fb_height, - const rfb::ScreenSet& layout) override; + void setScreenLayout(int fb_width, int fb_height, + const rfb::ScreenSet& layout) override; void handleClipboardRequest() override; void handleClipboardAnnounce(bool available) override; void handleClipboardData(const char* data) override; diff --git a/unix/xserver/hw/vnc/XserverDesktop.cc b/unix/xserver/hw/vnc/XserverDesktop.cc index 5e8164274f..828cf642ac 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.cc +++ b/unix/xserver/hw/vnc/XserverDesktop.cc @@ -482,8 +482,8 @@ void XserverDesktop::pointerEvent(const core::Point& pos, vncPointerButtonAction(buttonMask); } -unsigned int XserverDesktop::setScreenLayout(int fb_width, int fb_height, - const rfb::ScreenSet& layout) +void XserverDesktop::setScreenLayout(int fb_width, int fb_height, + const rfb::ScreenSet& layout) { unsigned int result; @@ -494,7 +494,7 @@ unsigned int XserverDesktop::setScreenLayout(int fb_width, int fb_height, // can be corner cases where we don't get feedback from the X core refreshScreenLayout(); - return result; + server->setScreenLayoutDone(result); } void XserverDesktop::frameTick(uint64_t msc) diff --git a/unix/xserver/hw/vnc/XserverDesktop.h b/unix/xserver/hw/vnc/XserverDesktop.h index 036cf9d201..b8b732c921 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.h +++ b/unix/xserver/hw/vnc/XserverDesktop.h @@ -100,8 +100,8 @@ class XserverDesktop : public rfb::SDesktop, public rfb::FullFramePixelBuffer, void pointerEvent(const core::Point& pos, uint16_t buttonMask) override; void keyEvent(uint32_t keysym, uint32_t keycode, bool down) override; - unsigned int setScreenLayout(int fb_width, int fb_height, - const rfb::ScreenSet& layout) override; + void setScreenLayout(int fb_width, int fb_height, + const rfb::ScreenSet& layout) override; void frameTick(uint64_t msc) override; void handleClipboardRequest() override; void handleClipboardAnnounce(bool available) override; diff --git a/win/rfb_win32/SDisplay.cxx b/win/rfb_win32/SDisplay.cxx index de986ead1c..ac5bc8e5c5 100644 --- a/win/rfb_win32/SDisplay.cxx +++ b/win/rfb_win32/SDisplay.cxx @@ -170,6 +170,10 @@ void SDisplay::queryConnection(network::Socket* sock, server->approveConnection(sock, true); } +void SDisplay::setScreenLayout(int /*fb_width*/, int /*fb_height*/, + const rfb::ScreenSet& /*layout*/) { + server->setScreenLayoutDone(rfb::resultProhibited); +} void SDisplay::startCore() { diff --git a/win/rfb_win32/SDisplay.h b/win/rfb_win32/SDisplay.h index e116b483a9..2c5deb25a7 100644 --- a/win/rfb_win32/SDisplay.h +++ b/win/rfb_win32/SDisplay.h @@ -79,6 +79,8 @@ namespace rfb { void terminate() override; void queryConnection(network::Socket* sock, const char* userName) override; + void setScreenLayout(int fb_width, int fb_height, + const ScreenSet& layout) override; void handleClipboardRequest() override; void handleClipboardAnnounce(bool available) override; void handleClipboardData(const char* data) override;