From cdac38f49333cd766634b76179205f906639697c Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Mon, 8 Jan 2024 16:38:19 -0800 Subject: [PATCH] Check for seekable stream for malformed uri handling - We must be able to seek to create a temporary file requried for uri handling - We should throw the same exception we did in v2.x - If compat mode is on, this will throw at open time (i.e. it will eagerly load everything) Fixes #1636 --- .../Features/PackageFeatureBase.cs | 11 +- .../Packaging/WriteableStreamExtensions.cs | 2 +- .../Resources/ExceptionMessages.Designer.cs | 9 ++ .../Resources/ExceptionMessages.resx | 3 + ...umentFormat.OpenXml.Framework.Tests.csproj | 1 + .../IFile.cs | 4 + .../TemporaryFile.cs | 4 + .../TestAssets.cs | 21 ++-- .../OpenXmlDomTest/DocumentOpenTests.cs | 100 ++++++++++++++++++ 9 files changed, 144 insertions(+), 11 deletions(-) diff --git a/src/DocumentFormat.OpenXml.Framework/Features/PackageFeatureBase.cs b/src/DocumentFormat.OpenXml.Framework/Features/PackageFeatureBase.cs index b3c5edec6..6c78b402b 100644 --- a/src/DocumentFormat.OpenXml.Framework/Features/PackageFeatureBase.cs +++ b/src/DocumentFormat.OpenXml.Framework/Features/PackageFeatureBase.cs @@ -243,7 +243,16 @@ protected override void Delete(string id) => Feature.Package.GetPart(Uri).DeleteRelationship(id); protected override IEnumerable GetRelationships() - => Feature.Package.GetPart(Uri).GetRelationships(); + { + try + { + return Feature.Package.GetPart(Uri).GetRelationships(); + } + catch (UriFormatException ex) + { + throw new OpenXmlPackageException(ExceptionMessages.MalformedUri, ex); + } + } } private abstract class RelationshipCollection : IRelationshipCollection diff --git a/src/DocumentFormat.OpenXml.Framework/Packaging/WriteableStreamExtensions.cs b/src/DocumentFormat.OpenXml.Framework/Packaging/WriteableStreamExtensions.cs index 94bfc092c..577a74b5d 100644 --- a/src/DocumentFormat.OpenXml.Framework/Packaging/WriteableStreamExtensions.cs +++ b/src/DocumentFormat.OpenXml.Framework/Packaging/WriteableStreamExtensions.cs @@ -15,7 +15,7 @@ internal static class WriteableStreamExtensions [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Disposable is registered with package")] public static bool EnableWriteableStream(this IFeatureCollection features) { - if (features.Get() is { Stream.CanWrite: false } feature && + if (features.Get() is { Stream: { CanWrite: false, CanSeek: true } } feature && features.Get() is { } packageFeature && packageFeature.Capabilities.HasFlagFast(PackageCapabilities.Reload)) { diff --git a/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.Designer.cs b/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.Designer.cs index 6eedab315..31c977c93 100644 --- a/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.Designer.cs +++ b/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.Designer.cs @@ -504,6 +504,15 @@ internal static string MainPartIsDifferent { } } + /// + /// Looks up a localized string similar to The package contains malformed URI relationships. Please ensure the package is opened with a stream (that is seekable) or a file path to have the SDK automatically handle these scenarios.. + /// + internal static string MalformedUri { + get { + return ResourceManager.GetString("MalformedUri", resourceCulture); + } + } + /// /// Looks up a localized string similar to An Audio / Video part shall not have implicit or explicit relationships to other parts defined by Open XML Standard.. /// diff --git a/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.resx b/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.resx index 3fa9aae68..b6a18e5d7 100644 --- a/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.resx +++ b/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.resx @@ -408,4 +408,7 @@ Namespace ids are only available for namespaces for Office 2021 and before. Please use prefixes or URLs instead. + + The package contains malformed URI relationships. Please ensure the package is opened with a stream (that is seekable) or a file path to have the SDK automatically handle these scenarios. + \ No newline at end of file diff --git a/test/DocumentFormat.OpenXml.Framework.Tests/DocumentFormat.OpenXml.Framework.Tests.csproj b/test/DocumentFormat.OpenXml.Framework.Tests/DocumentFormat.OpenXml.Framework.Tests.csproj index 4e6e2c276..dd185f6ac 100644 --- a/test/DocumentFormat.OpenXml.Framework.Tests/DocumentFormat.OpenXml.Framework.Tests.csproj +++ b/test/DocumentFormat.OpenXml.Framework.Tests/DocumentFormat.OpenXml.Framework.Tests.csproj @@ -21,6 +21,7 @@ + diff --git a/test/DocumentFormat.OpenXml.Tests.Assets/IFile.cs b/test/DocumentFormat.OpenXml.Tests.Assets/IFile.cs index 2ab208580..89e2d8072 100644 --- a/test/DocumentFormat.OpenXml.Tests.Assets/IFile.cs +++ b/test/DocumentFormat.OpenXml.Tests.Assets/IFile.cs @@ -11,5 +11,9 @@ public interface IFile : IDisposable string Path { get; } Stream Open(); + + bool IsEditable { get; } + + FileAccess Access { get; } } } diff --git a/test/DocumentFormat.OpenXml.Tests.Assets/TemporaryFile.cs b/test/DocumentFormat.OpenXml.Tests.Assets/TemporaryFile.cs index 6a47fca6a..b713841a5 100644 --- a/test/DocumentFormat.OpenXml.Tests.Assets/TemporaryFile.cs +++ b/test/DocumentFormat.OpenXml.Tests.Assets/TemporaryFile.cs @@ -16,6 +16,10 @@ private TemporaryFile(string path) public string Path { get; } + public bool IsEditable => true; + + public FileAccess Access => FileAccess.ReadWrite; + public Stream Open() => File.Open(Path, FileMode.OpenOrCreate, FileAccess.ReadWrite); public void Dispose() diff --git a/test/DocumentFormat.OpenXml.Tests.Assets/TestAssets.cs b/test/DocumentFormat.OpenXml.Tests.Assets/TestAssets.cs index 9aa6b0e99..fe24eddd7 100644 --- a/test/DocumentFormat.OpenXml.Tests.Assets/TestAssets.cs +++ b/test/DocumentFormat.OpenXml.Tests.Assets/TestAssets.cs @@ -98,6 +98,10 @@ public MemoryFile(Stream stream, string path) public string Path { get; } + public bool IsEditable => _stream.CanWrite; + + public FileAccess Access => _stream.CanWrite ? FileAccess.ReadWrite : FileAccess.Read; + public Stream Open() => _stream; public void Dispose() @@ -108,23 +112,22 @@ public void Dispose() private class CopiedFile : IFile { - private readonly FileAccess _access; - public CopiedFile(Stream stream, string extension, FileAccess access) { Path = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"{Guid.NewGuid()}{extension}"); + Access = access; - _access = access; - - using (var fs = File.OpenWrite(Path)) - { - stream.CopyTo(fs); - } + using var fs = File.OpenWrite(Path); + stream.CopyTo(fs); } public string Path { get; } - public Stream Open() => File.Open(Path, FileMode.Open, _access); + public bool IsEditable => Access == FileAccess.ReadWrite; + + public FileAccess Access { get; } + + public Stream Open() => File.Open(Path, FileMode.Open, Access); public void Dispose() { diff --git a/test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/DocumentOpenTests.cs b/test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/DocumentOpenTests.cs index 2d761fe2c..fe307a6e4 100644 --- a/test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/DocumentOpenTests.cs +++ b/test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/DocumentOpenTests.cs @@ -1,14 +1,18 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using DocumentFormat.OpenXml.Framework; using DocumentFormat.OpenXml.Packaging; using DocumentFormat.OpenXml.Spreadsheet; using DocumentFormat.OpenXml.Validation; using System; using System.Collections.Generic; using System.IO; +using System.Linq; using Xunit; +using static DocumentFormat.OpenXml.Tests.TestAssets; + namespace DocumentFormat.OpenXml.Tests { public class DocumentOpenTests @@ -35,5 +39,101 @@ public void ThrowIfFileCannotBeFound() Assert.Throws(() => PresentationDocument.Open(NonExistantFile, true, new OpenSettings())); Assert.Throws(() => PresentationDocument.Open(NonExistantFile, false, new OpenSettings())); } + + [Theory] + [InlineData(FileAccess.Read)] + [InlineData(FileAccess.ReadWrite)] + public void RewriteMalformedUriLong(FileAccess access) + { + // Arrange + const string ExpectedMalformedUri = "mailto:test@test.com;%20test2@test.com;%252test3@test.com;%20test3@test.com;%20test4@test.com;%20test5@test.com?subject=Unsubscribe%20Request&body=Please%20unsubscribe%20me%20from%20all%20future%20communications"; + const string Id = "rId1"; + + using var file = OpenFile(TestFiles.Malformed_uri_long_xlsx, access); + using var stream = file.Open(); + using var doc = SpreadsheetDocument.Open(stream, isEditable: file.IsEditable); + + // Act + var worksheetPart = doc.WorkbookPart.WorksheetParts.Single(); + var hyperlinkRelationship = worksheetPart.HyperlinkRelationships.Single(); + var worksheet = Assert.IsType(worksheetPart.RootElement); + var hyperlink = Assert.Single(worksheet.Descendants()); + + // Assert + Assert.Equal(Id, hyperlinkRelationship.Id); + Assert.Equal(Id, hyperlink.Id); + Assert.Equal(ExpectedMalformedUri, hyperlinkRelationship.Uri.ToString()); + } + + [Theory] + [InlineData(FileAccess.Read)] + [InlineData(FileAccess.ReadWrite)] + public void RewriteMalformedUri(FileAccess access) + { + // Arrange + const string Id = "rId1"; + + using var file = OpenFile(TestFiles.Malformed_uri_xlsx, access); + using var stream = file.Open(); + using var doc = SpreadsheetDocument.Open(stream, isEditable: file.IsEditable); + + // Act + var worksheetPart = doc.WorkbookPart.WorksheetParts.Single(); + var hyperlinkRelationship = worksheetPart.HyperlinkRelationships.Single(); + var worksheet = Assert.IsType(worksheetPart.RootElement); + var hyperlink = Assert.Single(worksheet.Descendants()); + + // Assert + Assert.Equal(Id, hyperlink.Id); + Assert.Equal(Id, hyperlinkRelationship.Id); + Assert.Equal("mailto:one@", hyperlinkRelationship.Uri.ToString()); + } + + [Fact] + public void NonSeekableRewriteMalformedUri() + { + // Arrange + using var stream = GetStream(TestFiles.Malformed_uri_xlsx); + using var doc = SpreadsheetDocument.Open(new NonSeekableStream(stream), isEditable: false); + + // Act/Assert + var worksheetPart = doc.WorkbookPart.WorksheetParts.Single(); + var exception = Assert.Throws(() => worksheetPart.HyperlinkRelationships.Single()); + + Assert.IsType(exception.InnerException); + } + + [Fact] + public void NonSeekableRewriteMalformedUriCompatMode() + { + // Arrange + using var stream = GetStream(TestFiles.Malformed_uri_xlsx); + + // Act/Assert + var exception = Assert.Throws(() => SpreadsheetDocument.Open(new NonSeekableStream(stream), isEditable: false, new OpenSettings { CompatibilityLevel = CompatibilityLevel.Version_2_20 })); + Assert.IsType(exception.InnerException); + } + + private sealed class NonSeekableStream : DelegatingStream + { + public NonSeekableStream(Stream innerStream) + : base(innerStream) + { + } + + public override bool CanSeek => false; + + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public override long Length => throw new NotSupportedException(); + + public override void SetLength(long value) => throw new NotSupportedException(); + } } }