diff --git a/client/vnc/server/agent_ipc.go b/client/vnc/server/agent_ipc.go index 594c8ca96..c0235378a 100644 --- a/client/vnc/server/agent_ipc.go +++ b/client/vnc/server/agent_ipc.go @@ -58,6 +58,17 @@ func (s *Server) handleServiceConnection(conn net.Conn, sa sessionAgent) { return } + // Service mode always attaches to the agent manager's active session. + // Honouring a requested session ID would mean routing to a specific + // desktop, which is not implemented; accepting the field anyway would + // silently hand the caller a different desktop from the one it asked for. + if header.sessionID != 0 { + rejectConnection(conn, codeMessage(RejectCodeBadRequest, + fmt.Sprintf("targeting session %d is not supported; omit the session id to attach to the active session", header.sessionID))) + connLog.Infof("VNC connection rejected: requested session %d, only the active session is supported", header.sessionID) + return + } + authedLog, sessionUserID, ok := s.authorizeSession(conn, header, connLog) if !ok { authedLog.Info("VNC connection rejected: auth failed") diff --git a/client/vnc/server/session.go b/client/vnc/server/session.go index 3eeb3056d..0578cd878 100644 --- a/client/vnc/server/session.go +++ b/client/vnc/server/session.go @@ -123,6 +123,10 @@ type session struct { // compositing falls back to a no-op (capturer cannot supply a sprite // or position). One line per session is enough to point at the cause. cursorWarnOnce sync.Once + // cursorSkipOnce does the same for the Cursor pseudo-encoding path, which + // has several ways to decline to send a rect and used to take all of them + // silently. + cursorSkipOnce sync.Once // clientJPEGQuality and clientZlibLevel hold the 0..9 levels the client // advertised via the QualityLevel / CompressLevel pseudo-encodings, or // -1 when the client has not expressed a preference. Applied to the diff --git a/client/vnc/server/session_cursor.go b/client/vnc/server/session_cursor.go index c791c70ce..c592295bf 100644 --- a/client/vnc/server/session_cursor.go +++ b/client/vnc/server/session_cursor.go @@ -23,11 +23,23 @@ func (s *session) pendingCursorRect(pf clientPixelFormat) []byte { composite := s.showRemoteCursor lastSerial := s.lastCursorSerial s.encMu.RUnlock() + // Each way out of here means the client gets no cursor, and they used to be + // indistinguishable from the outside: "no cursor on this platform" looked + // the same whether the client never asked for the encoding, the capturer + // cannot produce one, or the sprite failed to encode. Say which, once per + // session, so the next report of a missing cursor names its own cause. if !supported || failed || composite { + s.cursorSkipOnce.Do(func() { + s.log.Debugf("no cursor rect: client_requested=%v source_failed=%v compositing=%v", + supported, failed, composite) + }) return nil } src, ok := s.capturer.(cursorSource) if !ok { + s.cursorSkipOnce.Do(func() { + s.log.Debugf("no cursor rect: capturer %T reports no cursor source", s.capturer) + }) return nil } img, hotX, hotY, serial, err := src.Cursor() @@ -38,11 +50,22 @@ func (s *session) pendingCursorRect(pf clientPixelFormat) []byte { s.log.Debugf("cursor source unavailable: %v", err) return nil } - if img == nil || serial == lastSerial { + if img == nil { + s.cursorSkipOnce.Do(func() { + s.log.Debug("no cursor rect: capturer returned no sprite") + }) + return nil + } + if serial == lastSerial { return nil } buf := encodeCursorPseudoRect(img, hotX, hotY, pf) if buf == nil { + s.cursorSkipOnce.Do(func() { + b := img.Bounds() + s.log.Debugf("no cursor rect: sprite %dx%d stride=%d pix=%d could not be encoded", + b.Dx(), b.Dy(), img.Stride, len(img.Pix)) + }) return nil } // Re-check under the write lock so a cursor another goroutine already