Skip to content

Commit

Permalink
Check for seekable stream for malformed uri handling
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
twsouthwick committed Jan 9, 2024
1 parent dadb7e2 commit cdac38f
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,16 @@ protected override void Delete(string id)
=> Feature.Package.GetPart(Uri).DeleteRelationship(id);

protected override IEnumerable<PackageRelationship> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<IPackageStreamFeature>() is { Stream.CanWrite: false } feature &&
if (features.Get<IPackageStreamFeature>() is { Stream: { CanWrite: false, CanSeek: true } } feature &&
features.Get<IPackageFeature>() is { } packageFeature &&
packageFeature.Capabilities.HasFlagFast(PackageCapabilities.Reload))
{
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -408,4 +408,7 @@
<data name="NamespaceIdNotAvailable" xml:space="preserve">
<value>Namespace ids are only available for namespaces for Office 2021 and before. Please use prefixes or URLs instead.</value>
</data>
<data name="MalformedUri" xml:space="preserve">
<value>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.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<ItemGroup>
<ProjectReference Include="..\..\src\DocumentFormat.OpenXml.Framework\DocumentFormat.OpenXml.Framework.csproj" />
<ProjectReference Include="..\..\src\DocumentFormat.OpenXml\DocumentFormat.OpenXml.csproj" />
<ProjectReference Include="..\DocumentFormat.OpenXml.Tests.Assets\DocumentFormat.OpenXml.Tests.Assets.csproj" />
</ItemGroup>

</Project>
4 changes: 4 additions & 0 deletions test/DocumentFormat.OpenXml.Tests.Assets/IFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,9 @@ public interface IFile : IDisposable
string Path { get; }

Stream Open();

bool IsEditable { get; }

FileAccess Access { get; }
}
}
4 changes: 4 additions & 0 deletions test/DocumentFormat.OpenXml.Tests.Assets/TemporaryFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
21 changes: 12 additions & 9 deletions test/DocumentFormat.OpenXml.Tests.Assets/TestAssets.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
{
Expand Down
100 changes: 100 additions & 0 deletions test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/DocumentOpenTests.cs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -35,5 +39,101 @@ public void ThrowIfFileCannotBeFound()
Assert.Throws<FileNotFoundException>(() => PresentationDocument.Open(NonExistantFile, true, new OpenSettings()));
Assert.Throws<FileNotFoundException>(() => PresentationDocument.Open(NonExistantFile, false, new OpenSettings()));
}

[Theory]
[InlineData(FileAccess.Read)]
[InlineData(FileAccess.ReadWrite)]
public void RewriteMalformedUriLong(FileAccess access)
{
// Arrange
const string ExpectedMalformedUri = "mailto:[email protected];%[email protected];%[email protected];%[email protected];%[email protected];%[email protected]?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<Worksheet>(worksheetPart.RootElement);
var hyperlink = Assert.Single(worksheet.Descendants<Hyperlink>());

// 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<Worksheet>(worksheetPart.RootElement);
var hyperlink = Assert.Single(worksheet.Descendants<Hyperlink>());

// 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<OpenXmlPackageException>(() => worksheetPart.HyperlinkRelationships.Single());

Assert.IsType<UriFormatException>(exception.InnerException);
}

[Fact]
public void NonSeekableRewriteMalformedUriCompatMode()
{
// Arrange
using var stream = GetStream(TestFiles.Malformed_uri_xlsx);

// Act/Assert
var exception = Assert.Throws<OpenXmlPackageException>(() => SpreadsheetDocument.Open(new NonSeekableStream(stream), isEditable: false, new OpenSettings { CompatibilityLevel = CompatibilityLevel.Version_2_20 }));
Assert.IsType<UriFormatException>(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();
}
}
}

0 comments on commit cdac38f

Please sign in to comment.