fix: pr review

docs/zh/references/lan-transfer-protocol.md

@@ -222,52 +222,6 @@ function sendControlMessage(socket: Socket, message: object): void {
 {"type":"message_type",...其他字段...}\n
 ```
 
-### 4.3 消息发送
-
-```typescript
-function sendMessage(socket: Socket, message: object): void {
-  const payload = JSON.stringify(message);
-  socket.write(`${payload}\n`);
-}
-```
-
-### 4.4 消息接收与解析
-
-```typescript
-let buffer = "";
-
-socket.on("data", (chunk: Buffer) => {
-  buffer += chunk.toString("utf8");
-
-  let newlineIndex = buffer.indexOf("\n");
-  while (newlineIndex !== -1) {
-    const line = buffer.slice(0, newlineIndex).trim();
-    buffer = buffer.slice(newlineIndex + 1);
-
-    if (line.length > 0) {
-      const message = JSON.parse(line);
-      handleMessage(message);
-    }
-
-    newlineIndex = buffer.indexOf("\n");
-  }
-});
-```
-
-### 4.5 消息类型汇总
-
-| 类型             | 方向            | 用途         |
-| ---------------- | --------------- | ------------ |
-| `handshake`      | Client → Server | 握手请求     |
-| `handshake_ack`  | Server → Client | 握手响应     |
-| `ping`           | Client → Server | 心跳请求     |
-| `pong`           | Server → Client | 心跳响应     |
-| `file_start`     | Client → Server | 开始文件传输 |
-| `file_start_ack` | Server → Client | 文件传输确认 |
-| `file_chunk`     | Client → Server | 文件数据块（流式，无 per-chunk ACK） |
-| `file_end`       | Client → Server | 文件传输结束 |
-| `file_complete`  | Server → Client | 传输完成结果 |
-
 ---
 
 ## 5. 文件传输协议
@@ -802,7 +756,7 @@ class FileReceiver {
 
 ## 附录 A：TypeScript 类型定义
 
-完整的类型定义位于 `src/types/lanTransfer.ts`：
+完整的类型定义位于 `packages/shared/config/types.ts`：
 
 ```typescript
 // 握手消息

packages/shared/config/types.ts

@@ -161,7 +161,7 @@ export const LAN_TRANSFER_MAX_FILE_SIZE = 500 * 1024 * 1024 // 500MB
 export const LAN_TRANSFER_COMPLETE_TIMEOUT_MS = 60_000 // 60s - wait for file_complete after file_end
 export const LAN_TRANSFER_GLOBAL_TIMEOUT_MS = 10 * 60 * 1000 // 10 minutes - global transfer timeout
 
-// Binary protocol constants (v3)
+// Binary protocol constants (v1)
 export const LAN_TRANSFER_PROTOCOL_VERSION = '1'
 export const LAN_BINARY_FRAME_MAGIC = 0x4353 // "CS" as uint16
 export const LAN_BINARY_TYPE_FILE_CHUNK = 0x01
@@ -182,7 +182,7 @@ export type LanFileStartMessage = {
 
 /**
  * File chunk data (JSON format)
- * @deprecated Use binary frame format in protocol v2. This type is kept for reference only.
+ * @deprecated Use binary frame format in protocol v1. This type is kept for reference only.
  */
 export type LanFileChunkMessage = {
   type: 'file_chunk'
@@ -217,7 +217,7 @@ export type LanFileStartAckMessage = {
 
 /**
  * Acknowledgment of file chunk received
- * @deprecated Protocol v3 uses streaming mode without per-chunk acknowledgment.
+ * @deprecated Protocol v1 uses streaming mode without per-chunk acknowledgment.
  * This type is kept for backward compatibility reference only.
  */
 export type LanFileChunkAckMessage = {
@@ -235,7 +235,7 @@ export type LanFileCompleteMessage = {
   success: boolean
   filePath?: string // Path where file was saved on mobile
   error?: string
-  // v3 enhanced error diagnostics
+  // Enhanced error diagnostics
   errorCode?: 'CHECKSUM_MISMATCH' | 'INCOMPLETE_TRANSFER' | 'DISK_ERROR' | 'CANCELLED'
   receivedChunks?: number
   receivedBytes?: number

src/main/services/__tests__/BackupManager.deleteTempBackup.test.ts

@@ -0,0 +1,277 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+// Use vi.hoisted to define mocks that are available during hoisting
+const { mockLogger } = vi.hoisted(() => ({
+  mockLogger: {
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn()
+  }
+}))
+
+vi.mock('@logger', () => ({
+  loggerService: {
+    withContext: () => mockLogger
+  }
+}))
+
+vi.mock('electron', () => ({
+  app: {
+    getPath: vi.fn((key: string) => {
+      if (key === 'temp') return '/tmp'
+      if (key === 'userData') return '/mock/userData'
+      return '/mock/unknown'
+    })
+  }
+}))
+
+vi.mock('fs-extra', () => ({
+  default: {
+    pathExists: vi.fn(),
+    remove: vi.fn(),
+    ensureDir: vi.fn(),
+    copy: vi.fn(),
+    readdir: vi.fn(),
+    stat: vi.fn(),
+    readFile: vi.fn(),
+    writeFile: vi.fn(),
+    createWriteStream: vi.fn(),
+    createReadStream: vi.fn()
+  },
+  pathExists: vi.fn(),
+  remove: vi.fn(),
+  ensureDir: vi.fn(),
+  copy: vi.fn(),
+  readdir: vi.fn(),
+  stat: vi.fn(),
+  readFile: vi.fn(),
+  writeFile: vi.fn(),
+  createWriteStream: vi.fn(),
+  createReadStream: vi.fn()
+}))
+
+vi.mock('../WindowService', () => ({
+  windowService: {
+    getMainWindow: vi.fn()
+  }
+}))
+
+vi.mock('../WebDav', () => ({
+  default: vi.fn()
+}))
+
+vi.mock('../S3Storage', () => ({
+  default: vi.fn()
+}))
+
+vi.mock('../../utils', () => ({
+  getDataPath: vi.fn(() => '/mock/data')
+}))
+
+vi.mock('archiver', () => ({
+  default: vi.fn()
+}))
+
+vi.mock('node-stream-zip', () => ({
+  default: vi.fn()
+}))
+
+// Import after mocks
+import * as fs from 'fs-extra'
+
+import BackupManager from '../BackupManager'
+
+describe('BackupManager.deleteTempBackup - Security Tests', () => {
+  let backupManager: BackupManager
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    backupManager = new BackupManager()
+  })
+
+  describe('Normal Operations', () => {
+    it('should delete valid file in allowed directory', async () => {
+      vi.mocked(fs.pathExists).mockResolvedValue(true as never)
+      vi.mocked(fs.remove).mockResolvedValue(undefined as never)
+
+      const validPath = '/tmp/cherry-studio/lan-transfer/backup.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, validPath)
+
+      expect(result).toBe(true)
+      expect(fs.remove).toHaveBeenCalledWith(validPath)
+      expect(mockLogger.info).toHaveBeenCalledWith(expect.stringContaining('Deleted temp backup'))
+    })
+
+    it('should delete file in nested subdirectory', async () => {
+      vi.mocked(fs.pathExists).mockResolvedValue(true as never)
+      vi.mocked(fs.remove).mockResolvedValue(undefined as never)
+
+      const nestedPath = '/tmp/cherry-studio/lan-transfer/sub/dir/file.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, nestedPath)
+
+      expect(result).toBe(true)
+      expect(fs.remove).toHaveBeenCalledWith(nestedPath)
+    })
+
+    it('should return false when file does not exist', async () => {
+      vi.mocked(fs.pathExists).mockResolvedValue(false as never)
+
+      const missingPath = '/tmp/cherry-studio/lan-transfer/missing.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, missingPath)
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+    })
+  })
+
+  describe('Path Traversal Attacks', () => {
+    it('should block basic directory traversal attack (../../../../etc/passwd)', async () => {
+      const attackPath = '/tmp/cherry-studio/lan-transfer/../../../../etc/passwd'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, attackPath)
+
+      expect(result).toBe(false)
+      expect(fs.pathExists).not.toHaveBeenCalled()
+      expect(fs.remove).not.toHaveBeenCalled()
+      expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('outside temp directory'))
+    })
+
+    it('should block absolute path escape (/etc/passwd)', async () => {
+      const attackPath = '/etc/passwd'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, attackPath)
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+      expect(mockLogger.warn).toHaveBeenCalled()
+    })
+
+    it('should block traversal with multiple slashes', async () => {
+      const attackPath = '/tmp/cherry-studio/lan-transfer/../../../etc/passwd'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, attackPath)
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+    })
+
+    it('should block relative path traversal from current directory', async () => {
+      const attackPath = '../../../etc/passwd'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, attackPath)
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+    })
+
+    it('should block traversal to parent directory', async () => {
+      const attackPath = '/tmp/cherry-studio/lan-transfer/../backup/secret.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, attackPath)
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+    })
+  })
+
+  describe('Prefix Attacks', () => {
+    it('should block similar prefix attack (lan-transfer-evil)', async () => {
+      const attackPath = '/tmp/cherry-studio/lan-transfer-evil/file.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, attackPath)
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+      expect(mockLogger.warn).toHaveBeenCalled()
+    })
+
+    it('should block path without separator (lan-transferx)', async () => {
+      const attackPath = '/tmp/cherry-studio/lan-transferx'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, attackPath)
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+    })
+
+    it('should block different temp directory prefix', async () => {
+      const attackPath = '/tmp-evil/cherry-studio/lan-transfer/file.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, attackPath)
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+    })
+  })
+
+  describe('Error Handling', () => {
+    it('should return false and log error on permission denied', async () => {
+      vi.mocked(fs.pathExists).mockResolvedValue(true as never)
+      vi.mocked(fs.remove).mockRejectedValue(new Error('EACCES: permission denied') as never)
+
+      const validPath = '/tmp/cherry-studio/lan-transfer/file.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, validPath)
+
+      expect(result).toBe(false)
+      expect(mockLogger.error).toHaveBeenCalledWith(
+        expect.stringContaining('Failed to delete'),
+        expect.any(Error)
+      )
+    })
+
+    it('should return false on fs.pathExists error', async () => {
+      vi.mocked(fs.pathExists).mockRejectedValue(new Error('ENOENT') as never)
+
+      const validPath = '/tmp/cherry-studio/lan-transfer/file.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, validPath)
+
+      expect(result).toBe(false)
+      expect(mockLogger.error).toHaveBeenCalled()
+    })
+
+    it('should handle empty path string', async () => {
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, '')
+
+      expect(result).toBe(false)
+      expect(fs.remove).not.toHaveBeenCalled()
+    })
+  })
+
+  describe('Edge Cases', () => {
+    it('should allow deletion of the temp directory itself', async () => {
+      vi.mocked(fs.pathExists).mockResolvedValue(true as never)
+      vi.mocked(fs.remove).mockResolvedValue(undefined as never)
+
+      const tempDir = '/tmp/cherry-studio/lan-transfer'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, tempDir)
+
+      expect(result).toBe(true)
+      expect(fs.remove).toHaveBeenCalledWith(tempDir)
+    })
+
+    it('should handle path with trailing slash', async () => {
+      vi.mocked(fs.pathExists).mockResolvedValue(true as never)
+      vi.mocked(fs.remove).mockResolvedValue(undefined as never)
+
+      const pathWithSlash = '/tmp/cherry-studio/lan-transfer/sub/'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, pathWithSlash)
+
+      // path.normalize removes trailing slash
+      expect(result).toBe(true)
+    })
+
+    it('should handle file with special characters in name', async () => {
+      vi.mocked(fs.pathExists).mockResolvedValue(true as never)
+      vi.mocked(fs.remove).mockResolvedValue(undefined as never)
+
+      const specialPath = '/tmp/cherry-studio/lan-transfer/file with spaces & (special).zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, specialPath)
+
+      expect(result).toBe(true)
+      expect(fs.remove).toHaveBeenCalled()
+    })
+
+    it('should handle path with double slashes', async () => {
+      vi.mocked(fs.pathExists).mockResolvedValue(true as never)
+      vi.mocked(fs.remove).mockResolvedValue(undefined as never)
+
+      const doubleSlashPath = '/tmp/cherry-studio//lan-transfer//file.zip'
+      const result = await backupManager.deleteTempBackup({} as Electron.IpcMainInvokeEvent, doubleSlashPath)
+
+      // path.normalize handles double slashes
+      expect(result).toBe(true)
+    })
+  })
+})

src/main/services/__tests__/LocalTransferService.test.ts

@@ -0,0 +1,481 @@
+import { EventEmitter } from 'events'
+import { afterEach, beforeEach, describe, expect, it, type Mock,vi } from 'vitest'
+
+// Create mock objects before vi.mock calls
+const mockLogger = {
+  info: vi.fn(),
+  warn: vi.fn(),
+  error: vi.fn()
+}
+
+let mockMainWindow: {
+  isDestroyed: Mock
+  webContents: { send: Mock }
+} | null = null
+
+let mockBrowser: EventEmitter & {
+  start: Mock
+  stop: Mock
+  removeAllListeners: Mock
+}
+
+let mockBonjour: {
+  find: Mock
+  destroy: Mock
+}
+
+// Mock dependencies before importing the service
+vi.mock('@logger', () => ({
+  loggerService: {
+    withContext: () => mockLogger
+  }
+}))
+
+vi.mock('../WindowService', () => ({
+  windowService: {
+    getMainWindow: vi.fn(() => mockMainWindow)
+  }
+}))
+
+vi.mock('bonjour-service', () => ({
+  default: vi.fn(() => mockBonjour)
+}))
+
+describe('LocalTransferService', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    vi.resetModules()
+
+    // Reset mock objects
+    mockMainWindow = {
+      isDestroyed: vi.fn(() => false),
+      webContents: { send: vi.fn() }
+    }
+
+    mockBrowser = Object.assign(new EventEmitter(), {
+      start: vi.fn(),
+      stop: vi.fn(),
+      removeAllListeners: vi.fn()
+    })
+
+    mockBonjour = {
+      find: vi.fn(() => mockBrowser),
+      destroy: vi.fn()
+    }
+  })
+
+  afterEach(() => {
+    vi.resetAllMocks()
+  })
+
+  describe('startDiscovery', () => {
+    it('should set isScanning to true and start browser', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      const state = localTransferService.startDiscovery()
+
+      expect(state.isScanning).toBe(true)
+      expect(state.lastScanStartedAt).toBeDefined()
+      expect(mockBonjour.find).toHaveBeenCalledWith({ type: 'cherrystudio', protocol: 'tcp' })
+      expect(mockBrowser.start).toHaveBeenCalled()
+    })
+
+    it('should clear services when resetList is true', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      // First, start discovery and add a service
+      localTransferService.startDiscovery()
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100'],
+        fqdn: 'test.local'
+      })
+
+      expect(localTransferService.getState().services).toHaveLength(1)
+
+      // Now restart with resetList
+      const state = localTransferService.startDiscovery({ resetList: true })
+
+      expect(state.services).toHaveLength(0)
+    })
+
+    it('should broadcast state after starting discovery', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      expect(mockMainWindow?.webContents.send).toHaveBeenCalled()
+    })
+
+    it('should handle browser.start() error', async () => {
+      mockBrowser.start.mockImplementation(() => {
+        throw new Error('Failed to start mDNS')
+      })
+
+      const { localTransferService } = await import('../LocalTransferService')
+
+      const state = localTransferService.startDiscovery()
+
+      expect(state.lastError).toBe('Failed to start mDNS')
+      expect(mockLogger.error).toHaveBeenCalled()
+    })
+  })
+
+  describe('stopDiscovery', () => {
+    it('should set isScanning to false and stop browser', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+      const state = localTransferService.stopDiscovery()
+
+      expect(state.isScanning).toBe(false)
+      expect(mockBrowser.stop).toHaveBeenCalled()
+    })
+
+    it('should handle browser.stop() error gracefully', async () => {
+      mockBrowser.stop.mockImplementation(() => {
+        throw new Error('Stop failed')
+      })
+
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      // Should not throw
+      expect(() => localTransferService.stopDiscovery()).not.toThrow()
+      expect(mockLogger.warn).toHaveBeenCalled()
+    })
+
+    it('should broadcast state after stopping', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+      vi.clearAllMocks()
+
+      localTransferService.stopDiscovery()
+
+      expect(mockMainWindow?.webContents.send).toHaveBeenCalled()
+    })
+  })
+
+  describe('browser events', () => {
+    it('should add service on "up" event', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100'],
+        fqdn: 'test.local',
+        type: 'cherrystudio',
+        protocol: 'tcp'
+      })
+
+      const state = localTransferService.getState()
+      expect(state.services).toHaveLength(1)
+      expect(state.services[0].name).toBe('Test Service')
+      expect(state.services[0].port).toBe(12345)
+      expect(state.services[0].addresses).toContain('192.168.1.100')
+    })
+
+    it('should remove service on "down" event', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      // Add service
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100'],
+        fqdn: 'test.local'
+      })
+
+      expect(localTransferService.getState().services).toHaveLength(1)
+
+      // Remove service
+      mockBrowser.emit('down', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        fqdn: 'test.local'
+      })
+
+      expect(localTransferService.getState().services).toHaveLength(0)
+      expect(mockLogger.info).toHaveBeenCalledWith(expect.stringContaining('removed'))
+    })
+
+    it('should set lastError on "error" event', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('error', new Error('Discovery failed'))
+
+      const state = localTransferService.getState()
+      expect(state.lastError).toBe('Discovery failed')
+      expect(mockLogger.error).toHaveBeenCalled()
+    })
+
+    it('should handle non-Error objects in error event', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('error', 'String error message')
+
+      const state = localTransferService.getState()
+      expect(state.lastError).toBe('String error message')
+    })
+  })
+
+  describe('getState', () => {
+    it('should return sorted services by name', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('up', {
+        name: 'Zebra Service',
+        host: 'host1',
+        port: 1001,
+        addresses: ['192.168.1.1']
+      })
+
+      mockBrowser.emit('up', {
+        name: 'Alpha Service',
+        host: 'host2',
+        port: 1002,
+        addresses: ['192.168.1.2']
+      })
+
+      const state = localTransferService.getState()
+      expect(state.services[0].name).toBe('Alpha Service')
+      expect(state.services[1].name).toBe('Zebra Service')
+    })
+
+    it('should include all state properties', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      const state = localTransferService.getState()
+
+      expect(state).toHaveProperty('services')
+      expect(state).toHaveProperty('isScanning')
+      expect(state).toHaveProperty('lastScanStartedAt')
+      expect(state).toHaveProperty('lastUpdatedAt')
+    })
+  })
+
+  describe('getPeerById', () => {
+    it('should return peer when exists', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100'],
+        fqdn: 'test.local'
+      })
+
+      const services = localTransferService.getState().services
+      const peer = localTransferService.getPeerById(services[0].id)
+
+      expect(peer).toBeDefined()
+      expect(peer?.name).toBe('Test Service')
+    })
+
+    it('should return undefined when peer does not exist', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      const peer = localTransferService.getPeerById('non-existent-id')
+
+      expect(peer).toBeUndefined()
+    })
+  })
+
+  describe('normalizeService', () => {
+    it('should deduplicate addresses', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100', '192.168.1.100', '10.0.0.1'],
+        referer: { address: '192.168.1.100' }
+      })
+
+      const services = localTransferService.getState().services
+      expect(services[0].addresses).toHaveLength(2)
+      expect(services[0].addresses).toContain('192.168.1.100')
+      expect(services[0].addresses).toContain('10.0.0.1')
+    })
+
+    it('should filter empty addresses', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100', '', null as any]
+      })
+
+      const services = localTransferService.getState().services
+      expect(services[0].addresses).toEqual(['192.168.1.100'])
+    })
+
+    it('should convert txt null/undefined values to empty strings', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100'],
+        txt: {
+          version: '1.0',
+          nullValue: null,
+          undefinedValue: undefined,
+          numberValue: 42
+        }
+      })
+
+      const services = localTransferService.getState().services
+      expect(services[0].txt).toEqual({
+        version: '1.0',
+        nullValue: '',
+        undefinedValue: '',
+        numberValue: '42'
+      })
+    })
+
+    it('should not include txt when empty', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100'],
+        txt: {}
+      })
+
+      const services = localTransferService.getState().services
+      expect(services[0].txt).toBeUndefined()
+    })
+  })
+
+  describe('dispose', () => {
+    it('should clean up all resources', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      mockBrowser.emit('up', {
+        name: 'Test Service',
+        host: 'localhost',
+        port: 12345,
+        addresses: ['192.168.1.100']
+      })
+
+      localTransferService.dispose()
+
+      expect(localTransferService.getState().services).toHaveLength(0)
+      expect(localTransferService.getState().isScanning).toBe(false)
+      expect(mockBrowser.removeAllListeners).toHaveBeenCalled()
+      expect(mockBonjour.destroy).toHaveBeenCalled()
+    })
+
+    it('should handle bonjour.destroy() error gracefully', async () => {
+      mockBonjour.destroy.mockImplementation(() => {
+        throw new Error('Destroy failed')
+      })
+
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      // Should not throw
+      expect(() => localTransferService.dispose()).not.toThrow()
+      expect(mockLogger.warn).toHaveBeenCalled()
+    })
+
+    it('should be safe to call multiple times', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+
+      expect(() => {
+        localTransferService.dispose()
+        localTransferService.dispose()
+      }).not.toThrow()
+    })
+  })
+
+  describe('broadcastState', () => {
+    it('should not throw when main window is null', async () => {
+      mockMainWindow = null
+
+      const { localTransferService } = await import('../LocalTransferService')
+
+      // Should not throw
+      expect(() => localTransferService.startDiscovery()).not.toThrow()
+    })
+
+    it('should not throw when main window is destroyed', async () => {
+      mockMainWindow = {
+        isDestroyed: vi.fn(() => true),
+        webContents: { send: vi.fn() }
+      }
+
+      const { localTransferService } = await import('../LocalTransferService')
+
+      // Should not throw
+      expect(() => localTransferService.startDiscovery()).not.toThrow()
+      expect(mockMainWindow.webContents.send).not.toHaveBeenCalled()
+    })
+  })
+
+  describe('restartBrowser', () => {
+    it('should destroy old bonjour instance to prevent socket leaks', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      // First start
+      localTransferService.startDiscovery()
+      expect(mockBonjour.destroy).not.toHaveBeenCalled()
+
+      // Restart - should destroy old instance
+      localTransferService.startDiscovery()
+      expect(mockBonjour.destroy).toHaveBeenCalled()
+    })
+
+    it('should remove all listeners from old browser', async () => {
+      const { localTransferService } = await import('../LocalTransferService')
+
+      localTransferService.startDiscovery()
+      localTransferService.startDiscovery()
+
+      expect(mockBrowser.removeAllListeners).toHaveBeenCalled()
+    })
+  })
+})

src/main/services/lanTransfer/LanTransferClientService.ts

@@ -331,11 +331,10 @@ class LanTransferClientService {
     logger.info('Connection lost, attempting to reconnect...')
     this.reconnectPromise = this.connectAndHandshake(this.lastConnectOptions)
       .then(() => {
-        this.reconnectPromise = null
+        // Handshake succeeded, connection restored
       })
-      .catch((error) => {
+      .finally(() => {
         this.reconnectPromise = null
-        throw error
       })
 
     await this.reconnectPromise
@@ -406,7 +405,14 @@ class LanTransferClientService {
 
   private attachSocketListeners(socket: Socket): void {
     this.dataHandler = createDataHandler((line) => this.handleControlLine(line))
-    socket.on('data', (chunk: Buffer) => this.dataHandler?.handleData(chunk))
+    socket.on('data', (chunk: Buffer) => {
+      try {
+        this.dataHandler?.handleData(chunk)
+      } catch (error) {
+        logger.error('Data handler error', error as Error)
+        void this.disconnect()
+      }
+    })
   }
 
   private handleControlLine(line: string): void {
@@ -419,14 +425,14 @@ class LanTransferClientService {
       logger.warn('Received invalid JSON control message', { line, consecutiveErrors: this.consecutiveJsonErrors })
 
       if (this.consecutiveJsonErrors >= LanTransferClientService.MAX_CONSECUTIVE_JSON_ERRORS) {
-        const message = `Protocol error: ${this.consecutiveJsonErrors} consecutive invalid messages`
+        const message = `Protocol error: ${this.consecutiveJsonErrors} consecutive invalid messages, disconnecting`
         logger.error(message)
         this.broadcastClientEvent({
           type: 'error',
           message,
           timestamp: Date.now()
         })
-        this.consecutiveJsonErrors = 0
+        void this.disconnect()
       }
       return
     }