Skip to content

Commit 14fb3a1

Browse files
adityasaadityasa
authored
Query : Fixes OFFSET, LIMIT, TOP data types to match those coming from query plan (#4939)
## Query : Fixes OFFSET, LIMIT, TOP data types to match those coming from query plan This change ensures that the data types of OFFSET, LIMIT and TOP fields on QueryInfo and HybridSearchQueryInfo objects match those coming from the query plan. Currently the queryplan/service uses ULONG which has a range of 0x00000000 to 0xFFFFFFFF. Since QueryInfo fields use int?, this causes an overflow for a value that's larger than 0x7FFFFFFF for any of these fields. ## Type of change Please delete options that are not relevant. - [x] Bug fix (non-breaking change which fixes an issue) - [] New feature (non-breaking change which adds functionality) - [] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [] This change requires a documentation update
1 parent 8471ede commit 14fb3a1

17 files changed

+1255
-59
lines changed

‎Microsoft.Azure.Cosmos/src/Query/Core/Pipeline/PipelineFactory.cs

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,19 +108,27 @@ public static TryCatch<IQueryPipelineStage> MonadicCreate(
108108
maxConcurrency: maxConcurrency);
109109

110110
if (hybridSearchQueryInfo.Skip != null)
111-
{
111+
{
112+
Debug.Assert(hybridSearchQueryInfo.Skip.Value <= int.MaxValue, "PipelineFactory Assert!", "Skip value must be <= int.MaxValue");
113+
114+
int skipCount = (int)hybridSearchQueryInfo.Skip.Value;
115+
112116
MonadicCreatePipelineStage monadicCreateSourceStage = monadicCreatePipelineStage;
113117
monadicCreatePipelineStage = (continuationToken) => SkipQueryPipelineStage.MonadicCreate(
114-
hybridSearchQueryInfo.Skip.Value,
118+
skipCount,
115119
continuationToken,
116120
monadicCreateSourceStage);
117121
}
118122

119123
if (hybridSearchQueryInfo.Take != null)
120-
{
124+
{
125+
Debug.Assert(hybridSearchQueryInfo.Take.Value <= int.MaxValue, "PipelineFactory Assert!", "Take value must be <= int.MaxValue");
126+
127+
int takeCount = (int)hybridSearchQueryInfo.Take.Value;
128+
121129
MonadicCreatePipelineStage monadicCreateSourceStage = monadicCreatePipelineStage;
122130
monadicCreatePipelineStage = (continuationToken) => TakeQueryPipelineStage.MonadicCreateLimitStage(
123-
hybridSearchQueryInfo.Take.Value,
131+
takeCount,
124132
requestContinuationToken,
125133
monadicCreateSourceStage);
126134
}
@@ -150,7 +158,7 @@ public static TryCatch<IQueryPipelineStage> MonadicCreate(
150158
long optimalPageSize = maxItemCount;
151159
if (queryInfo.HasOrderBy)
152160
{
153-
int top;
161+
uint top;
154162
if (queryInfo.HasTop && (queryInfo.Top.Value > 0))
155163
{
156164
top = queryInfo.Top.Value;
@@ -162,6 +170,11 @@ public static TryCatch<IQueryPipelineStage> MonadicCreate(
162170
else
163171
{
164172
top = 0;
173+
}
174+
175+
if (top > int.MaxValue)
176+
{
177+
throw new ArgumentOutOfRangeException(nameof(queryInfo.Top.Value));
165178
}
166179

167180
if (top > 0)
@@ -257,27 +270,39 @@ public static TryCatch<IQueryPipelineStage> MonadicCreate(
257270

258271
if (queryInfo.HasOffset)
259272
{
273+
Debug.Assert(queryInfo.Offset.Value <= int.MaxValue, "PipelineFactory Assert!", "Offset value must be <= int.MaxValue");
274+
275+
int offsetCount = (int)queryInfo.Offset.Value;
276+
260277
MonadicCreatePipelineStage monadicCreateSourceStage = monadicCreatePipelineStage;
261278
monadicCreatePipelineStage = (continuationToken) => SkipQueryPipelineStage.MonadicCreate(
262-
queryInfo.Offset.Value,
279+
offsetCount,
263280
continuationToken,
264281
monadicCreateSourceStage);
265282
}
266283

267284
if (queryInfo.HasLimit)
268285
{
286+
Debug.Assert(queryInfo.Limit.Value <= int.MaxValue, "PipelineFactory Assert!", "Limit value must be <= int.MaxValue");
287+
288+
int limitCount = (int)queryInfo.Limit.Value;
289+
269290
MonadicCreatePipelineStage monadicCreateSourceStage = monadicCreatePipelineStage;
270291
monadicCreatePipelineStage = (continuationToken) => TakeQueryPipelineStage.MonadicCreateLimitStage(
271-
queryInfo.Limit.Value,
292+
limitCount,
272293
continuationToken,
273294
monadicCreateSourceStage);
274295
}
275296

276297
if (queryInfo.HasTop)
277298
{
299+
Debug.Assert(queryInfo.Top.Value <= int.MaxValue, "PipelineFactory Assert!", "Top value must be <= int.MaxValue");
300+
301+
int topCount = (int)queryInfo.Top.Value;
302+
278303
MonadicCreatePipelineStage monadicCreateSourceStage = monadicCreatePipelineStage;
279304
monadicCreatePipelineStage = (continuationToken) => TakeQueryPipelineStage.MonadicCreateTopStage(
280-
queryInfo.Top.Value,
305+
topCount,
281306
continuationToken,
282307
monadicCreateSourceStage);
283308
}

‎Microsoft.Azure.Cosmos/src/Query/Core/Pipeline/Skip/SkipQueryPipelineStage.Client.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
namespace Microsoft.Azure.Cosmos.Query.Core.Pipeline.Skip
66
{
77
using System;
8-
using System.Collections.Generic;
8+
using System.Collections.Generic;
9+
using System.Diagnostics;
910
using System.Linq;
1011
using System.Threading;
1112
using System.Threading.Tasks;
@@ -32,7 +33,7 @@ private ClientSkipQueryPipelineStage(
3233
int offsetCount,
3334
CosmosElement continuationToken,
3435
MonadicCreatePipelineStage monadicCreatePipelineStage)
35-
{
36+
{
3637
if (monadicCreatePipelineStage == null)
3738
{
3839
throw new ArgumentNullException(nameof(monadicCreatePipelineStage));
@@ -120,7 +121,9 @@ public override async ValueTask<bool> MoveNextAsync(ITrace trace, CancellationTo
120121
IReadOnlyList<CosmosElement> documentsAfterSkip = sourcePage.Documents.Skip(this.skipCount).ToList();
121122

122123
int numberOfDocumentsSkipped = sourcePage.Documents.Count - documentsAfterSkip.Count;
123-
this.skipCount -= numberOfDocumentsSkipped;
124+
this.skipCount -= numberOfDocumentsSkipped;
125+
126+
Debug.Assert(this.skipCount >= 0, $"{nameof(SkipQueryPipelineStage)} Assert!", "this.skipCount should be greater than or equal to 0");
124127

125128
QueryState state;
126129
if ((sourcePage.State != null) && (sourcePage.DisallowContinuationTokenMessage == null))

‎Microsoft.Azure.Cosmos/src/Query/Core/Pipeline/Skip/SkipQueryPipelineStage.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ protected SkipQueryPipelineStage(
2222
long skipCount)
2323
: base(source)
2424
{
25-
if (skipCount > int.MaxValue)
25+
if (skipCount > int.MaxValue || skipCount < 0)
2626
{
2727
throw new ArgumentOutOfRangeException(nameof(skipCount));
2828
}

‎Microsoft.Azure.Cosmos/src/Query/Core/Pipeline/Take/TakeQueryPipelineStage.Client.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ private ClientTakeQueryPipelineStage(
3838
{
3939
if (limitCount < 0)
4040
{
41-
throw new ArgumentException($"{nameof(limitCount)}: {limitCount} must be a non negative number.");
41+
throw new ArgumentException($"{nameof(limitCount)}: {limitCount} must be a non negative number.");
4242
}
4343

4444
if (monadicCreatePipelineStage == null)
@@ -256,7 +256,7 @@ private sealed class LimitContinuationToken : TakeContinuationToken
256256
/// <param name="limit">The limit to the number of document drained for the remainder of the query.</param>
257257
/// <param name="sourceToken">The continuation token for the source component of the query.</param>
258258
public LimitContinuationToken(int limit, string sourceToken)
259-
{
259+
{
260260
if (limit < 0)
261261
{
262262
throw new ArgumentException($"{nameof(limit)} must be a non negative number.");
@@ -330,7 +330,7 @@ private sealed class TopContinuationToken : TakeContinuationToken
330330
/// <param name="top">The limit to the number of document drained for the remainder of the query.</param>
331331
/// <param name="sourceToken">The continuation token for the source component of the query.</param>
332332
public TopContinuationToken(int top, string sourceToken)
333-
{
333+
{
334334
this.Top = top;
335335
this.SourceToken = sourceToken;
336336
}

‎Microsoft.Azure.Cosmos/src/Query/Core/Pipeline/Take/TakeQueryPipelineStage.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ protected TakeQueryPipelineStage(
1919
IQueryPipelineStage source,
2020
int takeCount)
2121
: base(source)
22-
{
22+
{
2323
this.takeCount = takeCount;
2424
}
2525

‎Microsoft.Azure.Cosmos/src/Query/Core/QueryPlan/HybridSearchQueryInfo.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ public QueryInfo ProjectionQueryInfo
3838
}
3939

4040
[JsonProperty("skip")]
41-
public int? Skip
41+
public uint? Skip
4242
{
4343
get;
4444
set;
4545
}
4646

4747
[JsonProperty("take")]
48-
public int? Take
48+
public uint? Take
4949
{
5050
get;
5151
set;

‎Microsoft.Azure.Cosmos/src/Query/Core/QueryPlan/QueryInfo.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,21 @@ public DistinctQueryType DistinctType
2424
}
2525

2626
[JsonProperty("top")]
27-
public int? Top
27+
public uint? Top
2828
{
2929
get;
3030
set;
3131
}
3232

3333
[JsonProperty("offset")]
34-
public int? Offset
34+
public uint? Offset
3535
{
3636
get;
3737
set;
3838
}
3939

4040
[JsonProperty("limit")]
41-
public int? Limit
41+
public uint? Limit
4242
{
4343
get;
4444
set;

‎Microsoft.Azure.Cosmos/src/Query/Core/QueryPlan/QueryPartitionProvider.cs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,56 @@ internal TryCatch<PartitionedQueryExecutionInfoInternal> TryGetPartitionedQueryE
318318
{
319319
DateParseHandling = DateParseHandling.None,
320320
MaxDepth = 64, // https://github.com/advisories/GHSA-5crp-9r3c-p9vr
321-
});
321+
});
322+
323+
if (!this.ValidateQueryExecutionInfo(queryInfoInternal, out ArgumentException innerException))
324+
{
325+
return TryCatch<PartitionedQueryExecutionInfoInternal>.FromException(
326+
new ExpectedQueryPartitionProviderException(
327+
serializedQueryExecutionInfo,
328+
innerException));
329+
}
322330

323331
return TryCatch<PartitionedQueryExecutionInfoInternal>.FromResult(queryInfoInternal);
332+
}
333+
334+
private bool ValidateQueryExecutionInfo(PartitionedQueryExecutionInfoInternal queryExecutionInfo, out ArgumentException innerException)
335+
{
336+
if (queryExecutionInfo.QueryInfo?.Limit.HasValue == true &&
337+
queryExecutionInfo.QueryInfo.Limit.Value > int.MaxValue)
338+
{
339+
innerException = new ArgumentOutOfRangeException("QueryInfo.Limit");
340+
return false;
341+
}
342+
343+
if (queryExecutionInfo.QueryInfo?.Offset.HasValue == true &&
344+
queryExecutionInfo.QueryInfo.Offset.Value > int.MaxValue)
345+
{
346+
innerException = new ArgumentOutOfRangeException("QueryInfo.Offset");
347+
return false;
348+
}
349+
350+
if (queryExecutionInfo.QueryInfo?.Top.HasValue == true &&
351+
queryExecutionInfo.QueryInfo.Top.Value > int.MaxValue)
352+
{
353+
innerException = new ArgumentOutOfRangeException("QueryInfo.Top");
354+
return false;
355+
}
356+
357+
if ((queryExecutionInfo.HybridSearchQueryInfo?.Skip ?? 0) > int.MaxValue)
358+
{
359+
innerException = new ArgumentOutOfRangeException("HybridSearchQueryInfo.Skip");
360+
return false;
361+
}
362+
363+
if ((queryExecutionInfo.HybridSearchQueryInfo?.Take ?? 0) > int.MaxValue)
364+
{
365+
innerException = new ArgumentOutOfRangeException("HybridSearchQueryInfo.Take");
366+
return false;
367+
}
368+
369+
innerException = null;
370+
return true;
324371
}
325372

326373
internal static TryCatch<IntPtr> TryCreateServiceProvider(string queryEngineConfiguration)

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Query/NegativeQueryTests.cs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,5 +101,47 @@ await this.CreateIngestQueryDeleteAsync(
101101
QueryTestsBase.NoDocuments,
102102
ImplementationAsync);
103103
}
104+
105+
[TestMethod]
106+
public async Task TestTopOffsetLimitClientRanges()
107+
{
108+
async Task ImplementationAsync(Container container, IReadOnlyList<CosmosObject> documents)
109+
{
110+
await QueryTestsBase.NoOp();
111+
112+
foreach((string parameterName, string query) in new[]
113+
{
114+
("QueryInfo.Offset", "SELECT c.name FROM c OFFSET 2147483648 LIMIT 10"),
115+
("QueryInfo.Limit", "SELECT c.name FROM c OFFSET 10 LIMIT 2147483648"),
116+
("QueryInfo.Top", "SELECT TOP 2147483648 c.name FROM c"),
117+
})
118+
try
119+
{
120+
List<Document> expectedValues = new List<Document>();
121+
FeedIterator<Document> resultSetIterator = container.GetItemQueryIterator<Document>(
122+
query,
123+
requestOptions: new QueryRequestOptions() { MaxConcurrency = 0 });
124+
125+
while (resultSetIterator.HasMoreResults)
126+
{
127+
expectedValues.AddRange(await resultSetIterator.ReadNextAsync());
128+
}
129+
130+
Assert.Fail("Expected to get an exception for this query.");
131+
}
132+
catch (CosmosException e)
133+
{
134+
Assert.IsTrue(e.StatusCode == HttpStatusCode.BadRequest);
135+
Assert.IsTrue(e.InnerException?.InnerException is ArgumentException ex &&
136+
ex.Message.Contains(parameterName));
137+
}
138+
}
139+
140+
await this.CreateIngestQueryDeleteAsync(
141+
ConnectionModes.Direct | ConnectionModes.Gateway,
142+
CollectionTypes.MultiPartition,
143+
QueryTestsBase.NoDocuments,
144+
ImplementationAsync);
145+
}
104146
}
105147
}

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Query/SkipTakeQueryTests.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,5 +185,43 @@ ORDER BY c.guid
185185
}
186186
}
187187
}
188+
189+
[TestMethod]
190+
public async Task TestTopOffsetLimitClientRanges()
191+
{
192+
async Task ImplementationAsync(Container container, IReadOnlyList<CosmosObject> documents)
193+
{
194+
await QueryTestsBase.NoOp();
195+
196+
foreach (string query in new[]
197+
{
198+
"SELECT c.name FROM c OFFSET 0 LIMIT 10",
199+
"SELECT c.name FROM c OFFSET 2147483647 LIMIT 10",
200+
"SELECT c.name FROM c OFFSET 10 LIMIT 0",
201+
"SELECT c.name FROM c OFFSET 10 LIMIT 2147483647",
202+
"SELECT TOP 0 c.name FROM c",
203+
"SELECT TOP 2147483647 c.name FROM c",
204+
})
205+
{
206+
List<CosmosElement> expectedValues = new List<CosmosElement>();
207+
FeedIterator<CosmosElement> resultSetIterator = container.GetItemQueryIterator<CosmosElement>(
208+
query,
209+
requestOptions: new QueryRequestOptions() { MaxConcurrency = 0 });
210+
211+
while (resultSetIterator.HasMoreResults)
212+
{
213+
expectedValues.AddRange(await resultSetIterator.ReadNextAsync());
214+
}
215+
216+
Assert.AreEqual(0, expectedValues.Count);
217+
}
218+
}
219+
220+
await this.CreateIngestQueryDeleteAsync(
221+
ConnectionModes.Direct | ConnectionModes.Gateway,
222+
CollectionTypes.MultiPartition,
223+
QueryTestsBase.NoDocuments,
224+
ImplementationAsync);
225+
}
188226
}
189227
}

0 commit comments

Comments
 (0)