Reject an unsupported VNC session id and say why a cursor rect was skipped

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")

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

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