fix: read output streams straight from native memory

RELEASENOTES.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 ### Fixed
+- Fixed a stream from `document.OpenOutputStream()` leaving the rendered document in `ArrayPool<byte>.Shared`. Only the byte-array reads were overridden, so span-shaped and asynchronous reads, and `CopyTo`, fell through to `Stream`, which stages the bytes through a rented array and returns it to the pool without clearing it. The next component in the process to rent from that pool could read the document back. The reads and the copy now go straight from native memory, which also removes a second copy of every byte: a 4 MiB span read measured 0.767 ms before and 0.274 ms after.
 - Fixed an input path in a subfolder aborting the process instead of compiling. The path was handed to Typst verbatim, and a Typst virtual path only accepts forward slashes, so on Windows an ordinary relative path such as `templates\letter.typ` panicked inside a native call and took the host process down with it. Paths are now resolved against the project root before being converted, and a path that genuinely leaves the root reports an error instead of panicking. Note that the root is matched against an absolute input path textually, so on Windows both have to be spelled with the same casing.
 - Fixed `ua-1` (PDF/UA-1, the accessibility standard) being rejected by `pdfStandards`. The accepted names are now taken from `typst_pdf::PdfStandard` itself, so every standard Typst supports is accepted, including ones added by later Typst releases. The documented `v-` prefix on plain PDF versions still works, and applies only to them: `v-a-2b` is not a spelling of `a-2b`.
 

src/typstsharp.tests/Tests.cs

@@ -1,3 +1,4 @@
+using System.Buffers;
 using System.Text;
 using UglyToad.PdfPig;
 using UglyToad.PdfPig.Content;
@@ -721,6 +722,175 @@ public async Task OutputStreamRejectsReadsAfterTheDocumentIsDisposed()
         await Assert.That(() => stream.Read(new byte[16].AsSpan())).Throws<ObjectDisposedException>();
     }
 
+    /// <summary>
+    /// A span read must not route through <see cref="Stream.Read(Span{byte})"/>, which serves the read
+    /// from an array rented from the process-wide <see cref="ArrayPool{T}"/> and returns it without
+    /// clearing it. That would leave the rendered document readable by the next unrelated component to
+    /// rent from the same pool, which is exactly what PooledBuffer goes out of its way to prevent.
+    /// </summary>
+    [Test]
+    [NotInParallel]
+    public async Task SpanReadLeavesNoDocumentBytesInTheSharedArrayPool()
+    {
+        using var compiler = TypstCompiler.FromSource("= Not for the next renter");
+        using var document = compiler.CompileToDocument();
+
+        int length = (int)document.GetOutputLength();
+
+        // Prime the pool so that a rent of this size is served from a known array rather than a
+        // fresh allocation. Nothing may await between here and the final rent, or the thread-local
+        // pool slot the priming lands in may not be the one the read and the check see.
+        byte[] primed = ArrayPool<byte>.Shared.Rent(length);
+        primed.AsSpan().Clear();
+        ArrayPool<byte>.Shared.Return(primed);
+
+        using (var stream = document.OpenOutputStream())
+        {
+            stream.ReadExactly(new byte[length]);
+        }
+
+        byte[] afterwards = ArrayPool<byte>.Shared.Rent(length);
+        bool sameArrayCameBack = ReferenceEquals(primed, afterwards);
+        bool carriesTheDocument = afterwards.AsSpan(0, 5).SequenceEqual("%PDF-"u8);
+        ArrayPool<byte>.Shared.Return(afterwards);
+
+        // Getting a different array back means the observation window was lost and the assertion
+        // below would hold for the wrong reason, so fail on that rather than passing green.
+        await Assert.That(sameArrayCameBack).IsTrue();
+        await Assert.That(carriesTheDocument).IsFalse();
+    }
+
+    /// <summary>
+    /// Copying to a response body is the likeliest use of this stream, and Stream.CopyTo stages
+    /// through its own array rented from the shared pool, so the copy needs the same guarantee as
+    /// the span read.
+    /// </summary>
+    [Test]
+    [NotInParallel]
+    public async Task CopyToLeavesNoDocumentBytesInTheSharedArrayPool()
+    {
+        using var compiler = TypstCompiler.FromSource("= Not for the next renter either");
+        using var document = compiler.CompileToDocument();
+
+        // 81920 is the staging buffer size Stream.CopyTo would rent.
+        const int copyBufferSize = 81920;
+        byte[] primed = ArrayPool<byte>.Shared.Rent(copyBufferSize);
+        primed.AsSpan().Clear();
+        ArrayPool<byte>.Shared.Return(primed);
+
+        using (var stream = document.OpenOutputStream())
+        {
+            stream.CopyTo(Stream.Null);
+        }
+
+        byte[] afterwards = ArrayPool<byte>.Shared.Rent(copyBufferSize);
+        bool sameArrayCameBack = ReferenceEquals(primed, afterwards);
+        bool carriesTheDocument = afterwards.AsSpan(0, 5).SequenceEqual("%PDF-"u8);
+        ArrayPool<byte>.Shared.Return(afterwards);
+
+        await Assert.That(sameArrayCameBack).IsTrue();
+        await Assert.That(carriesTheDocument).IsFalse();
+    }
+
+    /// <summary>
+    /// Position may legally be seeked past the end. The remaining length has to be compared before
+    /// it is narrowed to an int: an overshoot of just under 4 GiB wraps to a small positive count
+    /// and would read that far past the end of the native buffer.
+    /// </summary>
+    [Test]
+    public async Task ReadingPastTheEndOfTheBufferReturnsZero()
+    {
+        using var compiler = TypstCompiler.FromSource("= Seeked past the end");
+        using var document = compiler.CompileToDocument();
+
+        using var stream = document.OpenOutputStream();
+        var destination = new byte[4096];
+
+        stream.Position = stream.Length + 1;
+        await Assert.That(stream.Read(destination.AsSpan())).IsEqualTo(0);
+
+        stream.Position = stream.Length + int.MaxValue;
+        await Assert.That(stream.Read(destination.AsSpan())).IsEqualTo(0);
+
+        // The overshoot that wraps to a positive int when narrowed.
+        stream.Position = stream.Length + 4294966296;
+        await Assert.That(stream.Read(destination.AsSpan())).IsEqualTo(0);
+        await Assert.That(stream.Read(destination, 0, destination.Length)).IsEqualTo(0);
+    }
+
+    [Test]
+    public async Task CopyToAsyncMatchesOutputBytes()
+    {
+        using var compiler = TypstCompiler.FromSource("= Copied asynchronously");
+        using var document = compiler.CompileToDocument();
+        var expected = document.GetOutputBytes();
+
+        using var stream = document.OpenOutputStream();
+        using var copy = new MemoryStream();
+        await stream.CopyToAsync(copy);
+
+        await Assert.That(copy.ToArray().SequenceEqual(expected)).IsTrue();
+        await Assert.That(stream.Position).IsEqualTo((long)expected.Length);
+    }
+
+    [Test]
+    public async Task SpanReadReturnsTheWholeBufferAndThenReportsEndOfStream()
+    {
+        using var compiler = TypstCompiler.FromSource("= Read by span");
+        using var document = compiler.CompileToDocument();
+        var expected = document.GetOutputBytes();
+
+        using var stream = document.OpenOutputStream();
+
+        // A span larger than the document is the boundary worth pinning: the read must stop at the
+        // end of the buffer rather than at the end of the span.
+        var oversized = new byte[expected.Length + 64];
+        int read = stream.Read(oversized.AsSpan());
+
+        await Assert.That(read).IsEqualTo(expected.Length);
+        await Assert.That(oversized.AsSpan(0, expected.Length).SequenceEqual(expected)).IsTrue();
+        await Assert.That(stream.Read(oversized.AsSpan())).IsEqualTo(0);
+        await Assert.That(stream.Read(Span<byte>.Empty)).IsEqualTo(0);
+    }
+
+    [Test]
+    public async Task ChunkedSpanReadsReassembleTheDocument()
+    {
+        using var compiler = TypstCompiler.FromSource(TwoPageSource);
+        using var document = compiler.CompileToDocument();
+        var expected = document.GetOutputBytes();
+
+        using var stream = document.OpenOutputStream();
+        var reassembled = new byte[expected.Length];
+
+        // Chunks that do not divide the length evenly, so the final short read is covered too.
+        const int chunk = 1000;
+        int offset = 0;
+        int read;
+        while ((read = stream.Read(reassembled.AsSpan(offset, Math.Min(chunk, reassembled.Length - offset)))) > 0)
+        {
+            offset += read;
+        }
+
+        await Assert.That(offset).IsEqualTo(expected.Length);
+        await Assert.That(reassembled.SequenceEqual(expected)).IsTrue();
+    }
+
+    [Test]
+    public async Task AsyncReadMatchesOutputBytes()
+    {
+        using var compiler = TypstCompiler.FromSource("= Read asynchronously");
+        using var document = compiler.CompileToDocument();
+        var expected = document.GetOutputBytes();
+
+        using var stream = document.OpenOutputStream();
+        var destination = new byte[expected.Length];
+        int read = await stream.ReadAsync(destination.AsMemory());
+
+        await Assert.That(read).IsEqualTo(expected.Length);
+        await Assert.That(destination.SequenceEqual(expected)).IsTrue();
+    }
+
     [Test]
     public async Task OutputLengthMatchesOutputBytes()
     {

src/typstsharp/TypstDocument.cs

@@ -323,27 +323,62 @@ private unsafe void Free()
     /// <see cref="UnmanagedMemoryStream"/> would let the document be finalized, and its memory freed,
     /// while the stream was still being read.
     /// </summary>
-    private sealed unsafe class OutputStream : UnmanagedMemoryStream
+    private sealed class OutputStream : UnmanagedMemoryStream
     {
         private readonly TypstDocument _owner;
+        private readonly unsafe byte* _pointer;
 
-        internal OutputStream(TypstDocument owner, byte* pointer, long length)
+        internal unsafe OutputStream(TypstDocument owner, byte* pointer, long length)
             : base(pointer, length, length, FileAccess.Read)
         {
             _owner = owner;
+            _pointer = pointer;
         }
 
-        // Only the byte-array reads are overridden. Every other read path on Stream and
-        // UnmanagedMemoryStream, span and async alike, ends up calling one of these two, so this
-        // covers them all. Overriding Read(Span) as well would recurse: the UnmanagedMemoryStream
-        // override delegates to Stream.Read(Span) for any derived type, and that implementation
-        // calls back into Read(byte[]).
-        public override int Read(byte[] buffer, int offset, int count)
+        /// <summary>
+        /// Copies straight out of the native buffer, which is what keeps the span-shaped and
+        /// asynchronous read paths off the managed heap.
+        /// </summary>
+        /// <remarks>
+        /// <see cref="UnmanagedMemoryStream"/> only takes its own direct path when the runtime type is
+        /// exactly <see cref="UnmanagedMemoryStream"/>; for a derived type it defers to
+        /// <see cref="Stream.Read(Span{byte})"/>, which rents an array at least the size of the
+        /// caller's span from the shared <see cref="ArrayPool{T}"/>, reads into it, copies it out, and
+        /// returns it to the pool without clearing it. That costs a second copy of every byte and,
+        /// worse, leaves the rendered document in a process-wide pool for whatever rents from it next,
+        /// which is the very thing <c>PooledBuffer</c> clears its buffer to avoid. Reading the pointer
+        /// here sidesteps both. It does not recurse: recursion would need a call back into
+        /// <c>base.Read(Span)</c>.
+        /// </remarks>
+        public override unsafe int Read(Span<byte> buffer)
         {
             ObjectDisposedException.ThrowIf(_owner.IsDisposed, _owner);
-            int read = base.Read(buffer, offset, count);
+
+            long position = Position;
+
+            // The remaining length has to be tested before it is narrowed, not after. Position may
+            // legally be seeked past the end, and narrowing a large negative long wraps back to a
+            // positive int: an overshoot of just under 4 GiB would otherwise yield a small positive
+            // count and read that far past the end of the buffer.
+            long available = Length - position;
+            if (available <= 0 || buffer.IsEmpty)
+            {
+                return 0;
+            }
+
+            int count = (int)Math.Min((long)buffer.Length, available);
+            new ReadOnlySpan<byte>(_pointer + position, count).CopyTo(buffer);
+            Position = position + count;
             GC.KeepAlive(_owner);
-            return read;
+            return count;
+        }
+
+        // The array-shaped and asynchronous read paths funnel into Read(Span) above. ReadByte is the
+        // exception, and deliberately so: the base already serves it straight from the pointer.
+        public override int Read(byte[] buffer, int offset, int count)
+        {
+            ValidateBufferArguments(buffer, offset, count);
+            return Read(new Span<byte>(buffer, offset, count));
         }
 
         public override int ReadByte()
@@ -353,5 +388,63 @@ public override int ReadByte()
             GC.KeepAlive(_owner);
             return value;
         }
+
+        /// <summary>
+        /// Writes what is left of the buffer in a single call.
+        /// </summary>
+        /// <remarks>
+        /// Neither <see cref="UnmanagedMemoryStream"/> nor this type would otherwise override the
+        /// copy, so it would fall to <see cref="Stream"/>, which stages the document through an
+        /// 81920-byte array rented from the shared <see cref="ArrayPool{T}"/> and returned to it
+        /// uncleared. That is the same disclosure the span read above avoids, and copying a document
+        /// to a response body is the most likely thing a caller does with this stream.
+        /// </remarks>
+        public override unsafe void CopyTo(Stream destination, int bufferSize)
+        {
+            ValidateCopyToArguments(destination, bufferSize);
+            ObjectDisposedException.ThrowIf(_owner.IsDisposed, _owner);
+
+            long position = Position;
+            long remaining = Length - position;
+            while (remaining > 0)
+            {
+                // A single write per chunk, so only a buffer larger than int.MaxValue loops at all.
+                int chunk = (int)Math.Min(remaining, int.MaxValue);
+                destination.Write(new ReadOnlySpan<byte>(_pointer + position, chunk));
+                position += chunk;
+                remaining -= chunk;
+                Position = position;
+            }
+
+            GC.KeepAlive(_owner);
+        }
+
+        /// <inheritdoc cref="CopyTo(Stream, int)"/>
+        public override async Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken)
+        {
+            ValidateCopyToArguments(destination, bufferSize);
+            ObjectDisposedException.ThrowIf(_owner.IsDisposed, _owner);
+
+            long position = Position;
+            long remaining = Length - position;
+            while (remaining > 0)
+            {
+                int chunk = (int)Math.Min(remaining, int.MaxValue);
+                using var manager = CreateBufferManager(position, chunk);
+                await destination.WriteAsync(manager.Memory, cancellationToken).ConfigureAwait(false);
+                position += chunk;
+                remaining -= chunk;
+                Position = position;
+            }
+
+            GC.KeepAlive(_owner);
+        }
+
+        /// <summary>
+        /// Builds the <see cref="Memory{T}"/> view the asynchronous copy hands to the destination. It
+        /// needs its own method because a pointer cannot be used inside an asynchronous one.
+        /// </summary>
+        private unsafe NativeBufferMemoryManager CreateBufferManager(long position, int length) =>
+            new(_owner, _pointer + position, length);
     }
 }