Skip to content

Commit bfd8746

Browse files
adamnovaCopilotkirankumarkolli
authored
Client Encryption: Fixes MemoryTextReader.ReadToEnd returning empty string (#5801)
# Fixes `MemoryTextReader.ReadToEnd` returning an empty string ## The bug `Microsoft.Azure.Cosmos.Encryption.Custom.Transformation.MemoryTextReader.ReadToEnd()` set `this.pos = this.length` **before** computing the remaining slice as `(this.pos, this.length - this.pos)`. `this.length - this.pos` was therefore always zero, and the method returned an empty string regardless of the reader's contents or position. ```csharp // before this.pos = this.length; Memory<char> remaining = this.chars.Slice(this.pos, this.length - this.pos); // always empty slice return new string(remaining.ToArray()); ``` ## The fix Swap the ordering so the slice is computed from the current position first, then advance `pos`. Also replaces `Memory<char>.ToArray() + new string(char[])` with `ReadOnlySpan<char>.ToString()` to avoid an intermediate `char[]` allocation on every call. ```csharp // after int start = this.pos; int count = this.length - start; this.pos = this.length; if (count == 0) { return string.Empty; } return this.chars.Span.Slice(start, count).ToString(); ``` ## Impact on production code: none today The only in-tree caller of `MemoryTextReader` is `JObjectSqlSerializer.Deserialize` (line 114), which hands the reader to `Newtonsoft.Json.JsonTextReader`. `JsonTextReader` reads via `Read(buffer, index, count)` — not `ReadToEnd` — so this bug has no observable effect on the encrypt/decrypt paths today. However, `MemoryTextReader` derives from `TextReader` and any future consumer reaching for `ReadToEnd` would silently see an empty result. This fix makes the method behave as its public `TextReader` contract requires. ## Tests Adds `MemoryTextReaderTests.cs` with 23 cases covering: - The bug directly (`ReadToEnd_WhenFreshReader_ReturnsFullContent`, cross-validated against `StringReader`). - Edge cases: empty input, fully-consumed reader, double-`ReadToEnd`, partial read then `ReadToEnd`, Unicode input. - Argument validation: null buffer, negative index/count, count exceeding buffer. - Disposal guards: `Peek`/`Read`/`ReadLine`/`ReadToEnd` after `Close()` all throw. - Other previously-uncovered overrides: `Peek`, `Read()`, `Read(buffer,index,count)`, `ReadLine`. **Regression guard**: 6 of the new `ReadToEnd` tests fail against the buggy implementation and pass against the fixed one: ``` Failed ReadToEnd_WhenFreshReader_ReturnsFullContent Failed ReadToEnd_AfterPartialRead_ReturnsRemainderOnly Failed ReadToEnd_CalledTwice_SecondCallReturnsEmptyString Failed ReadToEnd_OnUnicodeContent_PreservesCharacters Failed ReadToEnd_MatchesStringReaderBehaviour Failed ReadToEnd_AfterPartialRead_MatchesStringReaderBehaviour Failed! - Failed: 6, Passed: 17, Skipped: 0, Total: 23 ``` After the fix: 23/23 pass; all 214 other encryption-custom tests pass (`net8.0`, `--filter "TestCategory!=Integration"`). ## Risks Minimal. The method had no observed callers relying on the broken behaviour (it was always returning `""` which no consumer is plausibly relying on). The fix is a 2-line swap plus a cheap allocation optimisation. No public API surface changes. ## Checklist - [x] Branch: `users/adamnova/memorytextreader-readtoend` - [x] PR title matches the required regex (`Client Encryption: Fixes MemoryTextReader.ReadToEnd returning empty string`) - [x] `Co-authored-by: Copilot` trailer on the commit - [x] No changes to `Directory.Build.props` / versioning / packaging - [x] Both `net8.0` and `netstandard2.0` targets build - [x] Unit tests added with regression coverage; full suite passes - [x] Integration tests (require Cosmos emulator) — not run Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Kiran Kumar Kolli <kirankk@microsoft.com>
1 parent dd06aae commit bfd8746

2 files changed

Lines changed: 254 additions & 2 deletions

File tree

Microsoft.Azure.Cosmos.Encryption.Custom/src/Transformation/MemoryTextReader.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,15 @@ public override string ReadToEnd()
103103
throw new InvalidOperationException("Reader is closed");
104104
}
105105

106+
int start = this.pos;
107+
int count = this.length - start;
106108
this.pos = this.length;
107-
Memory<char> remaining = this.chars.Slice(this.pos, this.length - this.pos);
108-
return new string(remaining.ToArray());
109+
if (count == 0)
110+
{
111+
return string.Empty;
112+
}
113+
114+
return this.chars.Span.Slice(start, count).ToString();
109115
}
110116

111117
public override string ReadLine()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
//------------------------------------------------------------
2+
// Copyright (c) Microsoft Corporation. All rights reserved.
3+
//------------------------------------------------------------
4+
namespace Microsoft.Azure.Cosmos.Encryption.Tests.Transformation
5+
{
6+
using System;
7+
using System.Collections.Generic;
8+
using System.IO;
9+
using Microsoft.Azure.Cosmos.Encryption.Custom.Transformation;
10+
using Microsoft.VisualStudio.TestTools.UnitTesting;
11+
12+
[TestClass]
13+
public class MemoryTextReaderTests
14+
{
15+
private static MemoryTextReader Create(string value)
16+
{
17+
return new MemoryTextReader(value.ToCharArray().AsMemory());
18+
}
19+
20+
[TestMethod]
21+
public void ReadToEnd_WhenFreshReader_ReturnsFullContent()
22+
{
23+
using MemoryTextReader reader = Create("hello world");
24+
25+
string result = reader.ReadToEnd();
26+
27+
Assert.AreEqual("hello world", result);
28+
}
29+
30+
[TestMethod]
31+
public void ReadToEnd_AfterPartialRead_ReturnsRemainderOnly()
32+
{
33+
using MemoryTextReader reader = Create("abcdef");
34+
_ = reader.Read();
35+
_ = reader.Read();
36+
37+
string result = reader.ReadToEnd();
38+
39+
Assert.AreEqual("cdef", result);
40+
}
41+
42+
[TestMethod]
43+
public void ReadToEnd_WhenEmptyInput_ReturnsEmptyString()
44+
{
45+
using MemoryTextReader reader = Create(string.Empty);
46+
47+
Assert.AreEqual(string.Empty, reader.ReadToEnd());
48+
}
49+
50+
[TestMethod]
51+
public void ReadToEnd_AfterFullyConsumed_ReturnsEmptyString()
52+
{
53+
using MemoryTextReader reader = Create("xyz");
54+
char[] buffer = new char[16];
55+
int n = reader.Read(buffer, 0, buffer.Length);
56+
Assert.AreEqual(3, n);
57+
58+
Assert.AreEqual(string.Empty, reader.ReadToEnd());
59+
}
60+
61+
[TestMethod]
62+
public void ReadToEnd_CalledTwice_SecondCallReturnsEmptyString()
63+
{
64+
using MemoryTextReader reader = Create("payload");
65+
66+
Assert.AreEqual("payload", reader.ReadToEnd());
67+
Assert.AreEqual(string.Empty, reader.ReadToEnd());
68+
}
69+
70+
[TestMethod]
71+
public void ReadToEnd_AfterClose_Throws()
72+
{
73+
MemoryTextReader reader = Create("data");
74+
reader.Close();
75+
76+
Assert.ThrowsException<InvalidOperationException>(() => reader.ReadToEnd());
77+
}
78+
79+
[TestMethod]
80+
public void ReadToEnd_OnUnicodeContent_PreservesCharacters()
81+
{
82+
string input = "héllo → 🙂 αβγ";
83+
using MemoryTextReader reader = Create(input);
84+
85+
Assert.AreEqual(input, reader.ReadToEnd());
86+
}
87+
88+
[TestMethod]
89+
public void ReadToEnd_MatchesStringReaderBehaviour()
90+
{
91+
string input = "line1\r\nline2\nline3";
92+
using MemoryTextReader memoryReader = Create(input);
93+
using StringReader stringReader = new (input);
94+
95+
Assert.AreEqual(stringReader.ReadToEnd(), memoryReader.ReadToEnd());
96+
}
97+
98+
[TestMethod]
99+
public void ReadToEnd_AfterPartialRead_MatchesStringReaderBehaviour()
100+
{
101+
string input = "abcdefghij";
102+
using MemoryTextReader memoryReader = Create(input);
103+
using StringReader stringReader = new (input);
104+
105+
char[] buffer = new char[4];
106+
int n1 = memoryReader.Read(buffer, 0, buffer.Length);
107+
int n2 = stringReader.Read(buffer, 0, buffer.Length);
108+
Assert.AreEqual(n2, n1);
109+
110+
Assert.AreEqual(stringReader.ReadToEnd(), memoryReader.ReadToEnd());
111+
}
112+
113+
[TestMethod]
114+
public void Peek_WhenFreshReader_ReturnsFirstChar_WithoutAdvancingPosition()
115+
{
116+
using MemoryTextReader reader = Create("abc");
117+
118+
Assert.AreEqual((int)'a', reader.Peek());
119+
Assert.AreEqual((int)'a', reader.Peek());
120+
Assert.AreEqual((int)'a', reader.Read());
121+
Assert.AreEqual((int)'b', reader.Peek());
122+
}
123+
124+
[TestMethod]
125+
public void Peek_AtEnd_ReturnsMinusOne()
126+
{
127+
using MemoryTextReader reader = Create("x");
128+
_ = reader.Read();
129+
130+
Assert.AreEqual(-1, reader.Peek());
131+
}
132+
133+
[TestMethod]
134+
public void Peek_WhenEmpty_ReturnsMinusOne()
135+
{
136+
using MemoryTextReader reader = Create(string.Empty);
137+
138+
Assert.AreEqual(-1, reader.Peek());
139+
}
140+
141+
[TestMethod]
142+
public void Peek_AfterClose_Throws()
143+
{
144+
MemoryTextReader reader = Create("x");
145+
reader.Close();
146+
147+
Assert.ThrowsException<InvalidOperationException>(() => reader.Peek());
148+
}
149+
150+
[TestMethod]
151+
public void Read_AfterClose_Throws()
152+
{
153+
MemoryTextReader reader = Create("x");
154+
reader.Close();
155+
156+
Assert.ThrowsException<InvalidOperationException>(() => reader.Read());
157+
}
158+
159+
[TestMethod]
160+
public void ReadBuffer_AfterClose_Throws()
161+
{
162+
MemoryTextReader reader = Create("x");
163+
reader.Close();
164+
165+
Assert.ThrowsException<InvalidOperationException>(
166+
() => reader.Read(new char[1], 0, 1));
167+
}
168+
169+
[TestMethod]
170+
public void ReadBuffer_NullBuffer_Throws()
171+
{
172+
using MemoryTextReader reader = Create("x");
173+
174+
Assert.ThrowsException<ArgumentNullException>(() => reader.Read(null, 0, 1));
175+
}
176+
177+
[TestMethod]
178+
public void ReadBuffer_NegativeIndex_Throws()
179+
{
180+
using MemoryTextReader reader = Create("x");
181+
182+
Assert.ThrowsException<ArgumentOutOfRangeException>(() => reader.Read(new char[4], -1, 1));
183+
}
184+
185+
[TestMethod]
186+
public void ReadBuffer_NegativeCount_Throws()
187+
{
188+
using MemoryTextReader reader = Create("x");
189+
190+
Assert.ThrowsException<ArgumentOutOfRangeException>(() => reader.Read(new char[4], 0, -1));
191+
}
192+
193+
[TestMethod]
194+
public void ReadBuffer_CountExceedsBuffer_Throws()
195+
{
196+
using MemoryTextReader reader = Create("x");
197+
198+
Assert.ThrowsException<ArgumentOutOfRangeException>(() => reader.Read(new char[4], 2, 10));
199+
}
200+
201+
[TestMethod]
202+
public void ReadBuffer_WhenCountLargerThanRemaining_ReturnsOnlyRemaining()
203+
{
204+
using MemoryTextReader reader = Create("abc");
205+
char[] dest = new char[16];
206+
207+
int n = reader.Read(dest, 2, 10);
208+
Assert.AreEqual(3, n);
209+
Assert.AreEqual('a', dest[2]);
210+
Assert.AreEqual('b', dest[3]);
211+
Assert.AreEqual('c', dest[4]);
212+
}
213+
214+
[TestMethod]
215+
public void ReadBuffer_WhenAtEnd_ReturnsZero()
216+
{
217+
using MemoryTextReader reader = Create("ab");
218+
_ = reader.ReadToEnd();
219+
220+
Assert.AreEqual(0, reader.Read(new char[4], 0, 4));
221+
}
222+
223+
[TestMethod]
224+
public void ReadLine_IteratesExpectedLines()
225+
{
226+
using MemoryTextReader reader = Create("a\r\nbc\nd");
227+
List<string> lines = new ();
228+
string line;
229+
while ((line = reader.ReadLine()) != null)
230+
{
231+
lines.Add(line);
232+
}
233+
234+
CollectionAssert.AreEqual(new[] { "a", "bc", "d" }, lines);
235+
}
236+
237+
[TestMethod]
238+
public void ReadLine_AfterClose_Throws()
239+
{
240+
MemoryTextReader reader = Create("x");
241+
reader.Close();
242+
243+
Assert.ThrowsException<InvalidOperationException>(() => reader.ReadLine());
244+
}
245+
}
246+
}

0 commit comments

Comments
 (0)