From cf4162e95d7e845690954a29384997dfe36810f5 Mon Sep 17 00:00:00 2001 From: Marnie0415 Date: Wed, 15 Jul 2026 06:57:30 +0800 Subject: [PATCH] fix: dispose backing FileStream on ExcelHandler/PowerPointHandler constructor failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExcelHandler and PowerPointHandler constructors create a backing FileStream but only catch OpenXmlPackageException (ExcelHandler) or no exceptions at all (PowerPointHandler). If PresentationDocument.Open or SpreadsheetDocument.Open fails for other reasons (IOException, UnauthorizedAccessException), the FileStream is never disposed — the file handle leaks until process exit. WordHandler already has the correct pattern: try/catch that disposes both _doc and _backingStream on failure. Apply the same cleanup to ExcelHandler (also disposing _filteredPackageStream for the bloat- filter path) and PowerPointHandler. Both handlers now use catch-all (matching WordHandler) so the original exception propagates directly — the factory's repair-and-retry paths (IsEncodingException, IsDanglingPartException) walk the exception chain and continue to work correctly. --- src/officecli/Handlers/ExcelHandler.cs | 13 ++++++++++--- src/officecli/Handlers/PowerPointHandler.cs | 20 +++++++++++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/officecli/Handlers/ExcelHandler.cs b/src/officecli/Handlers/ExcelHandler.cs index 3cd25716e..f07cd227d 100644 --- a/src/officecli/Handlers/ExcelHandler.cs +++ b/src/officecli/Handlers/ExcelHandler.cs @@ -96,10 +96,17 @@ public ExcelHandler(string filePath, bool editable) .Where(n => !string.IsNullOrEmpty(n)) ?? Enumerable.Empty(), StringComparer.OrdinalIgnoreCase); } - catch (DocumentFormat.OpenXml.Packaging.OpenXmlPackageException ex) + catch { - throw new InvalidOperationException( - $"Cannot open {Path.GetFileName(filePath)}: {ex.Message}", ex); + // A failed open must not leak the backing FileStream — the + // factory's repair-and-retry paths (FixXmlEncoding / + // StripDanglingPackageRels) reopen the file for in-place fixes + // and would hit "file is being used by another process". + // Matches the cleanup pattern in WordHandler's constructor. + _doc?.Dispose(); + _filteredPackageStream?.Dispose(); + _backingStream?.Dispose(); + throw; } } diff --git a/src/officecli/Handlers/PowerPointHandler.cs b/src/officecli/Handlers/PowerPointHandler.cs index 857907231..62eec0a0d 100644 --- a/src/officecli/Handlers/PowerPointHandler.cs +++ b/src/officecli/Handlers/PowerPointHandler.cs @@ -115,9 +115,23 @@ public PowerPointHandler(string filePath, bool editable) var share = editable ? FileShare.Read : FileShare.ReadWrite; var access = editable ? FileAccess.ReadWrite : FileAccess.Read; _backingStream = new FileStream(filePath, FileMode.Open, access, share); - _doc = PresentationDocument.Open(_backingStream, editable); - if (editable) - InitShapeIdCounter(); + try + { + _doc = PresentationDocument.Open(_backingStream, editable); + if (editable) + InitShapeIdCounter(); + } + catch + { + // A failed open must not leak the backing FileStream — the + // factory's repair-and-retry paths (FixXmlEncoding / + // StripDanglingPackageRels) reopen the file for in-place fixes + // and would hit "file is being used by another process". + // Matches the cleanup pattern in WordHandler's constructor. + _doc?.Dispose(); + _backingStream?.Dispose(); + throw; + } } ///