Skip to content

Commit 7d55af3

Browse files
michaelstaibmichaelstaib
authored
Ensure that query id has a valid format. (#6950)
1 parent a4d7d5e commit 7d55af3

27 files changed

+500
-154
lines changed

‎global.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
22
"sdk": {
3-
"version": "8.0.100",
4-
"rollForward": "latestMinor"
3+
"version": "8.0.100"
54
}
65
}

‎src/HotChocolate/AspNetCore/src/AspNetCore/ErrorHelper.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,10 @@ public static IQueryResult MultiPartRequestPreflightRequired()
6666
new Error(
6767
ErrorHelper_MultiPartRequestPreflightRequired,
6868
code: ErrorCodes.Server.MultiPartPreflightRequired));
69+
70+
public static GraphQLRequestException InvalidQueryIdFormat()
71+
=> new GraphQLRequestException(
72+
ErrorBuilder.New()
73+
.SetMessage("Invalid query id format.")
74+
.Build());
6975
}

‎src/HotChocolate/AspNetCore/src/AspNetCore/HttpGetMiddleware.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ private async Task HandleRequestAsync(HttpContext context)
127127
{
128128
try
129129
{
130-
request = _requestParser.ReadParamsRequest(context.Request.Query);
130+
request = _requestParser.ParseRequestFromParams(context.Request.Query);
131131
}
132132
catch (GraphQLRequestException ex)
133133
{

‎src/HotChocolate/AspNetCore/src/AspNetCore/HttpMultipartMiddleware.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public override async Task InvokeAsync(HttpContext context)
7171
}
7272
}
7373

74-
protected override async ValueTask<IReadOnlyList<GraphQLRequest>> GetRequestsFromBody(
74+
protected override async ValueTask<IReadOnlyList<GraphQLRequest>> ParseRequestsFromBody(
7575
HttpRequest httpRequest,
7676
CancellationToken cancellationToken)
7777
{
@@ -89,7 +89,7 @@ protected override async ValueTask<IReadOnlyList<GraphQLRequest>> GetRequestsFro
8989

9090
// Parse the string values of interest from the IFormCollection
9191
var multipartRequest = ParseMultipartRequest(form);
92-
var requests = RequestParser.ReadOperationsRequest(
92+
var requests = RequestParser.ParseRequest(
9393
multipartRequest.Operations);
9494

9595
foreach (var graphQLRequest in requests)

‎src/HotChocolate/AspNetCore/src/AspNetCore/HttpPostMiddleware.cs

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,17 @@
44

55
namespace HotChocolate.AspNetCore;
66

7-
public sealed class HttpPostMiddleware : HttpPostMiddlewareBase
8-
{
9-
public HttpPostMiddleware(
10-
HttpRequestDelegate next,
11-
IRequestExecutorResolver executorResolver,
12-
IHttpResponseFormatter responseFormatter,
13-
IHttpRequestParser requestParser,
14-
IServerDiagnosticEvents diagnosticEvents,
15-
string schemaName)
16-
: base(
17-
next,
18-
executorResolver,
19-
responseFormatter,
20-
requestParser,
21-
diagnosticEvents,
22-
schemaName)
23-
{
24-
}
25-
}
7+
public sealed class HttpPostMiddleware(
8+
HttpRequestDelegate next,
9+
IRequestExecutorResolver executorResolver,
10+
IHttpResponseFormatter responseFormatter,
11+
IHttpRequestParser requestParser,
12+
IServerDiagnosticEvents diagnosticEvents,
13+
string schemaName)
14+
: HttpPostMiddlewareBase(
15+
next,
16+
executorResolver,
17+
responseFormatter,
18+
requestParser,
19+
diagnosticEvents,
20+
schemaName);

‎src/HotChocolate/AspNetCore/src/AspNetCore/HttpPostMiddlewareBase.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ protected async Task HandleRequestAsync(HttpContext context)
111111
{
112112
try
113113
{
114-
requests = await GetRequestsFromBody(context.Request, context.RequestAborted);
114+
requests = await ParseRequestsFromBody(context.Request, context.RequestAborted);
115115
}
116116
catch (GraphQLRequestException ex)
117117
{
@@ -283,10 +283,10 @@ protected async Task HandleRequestAsync(HttpContext context)
283283
}
284284
}
285285

286-
protected virtual ValueTask<IReadOnlyList<GraphQLRequest>> GetRequestsFromBody(
286+
protected virtual ValueTask<IReadOnlyList<GraphQLRequest>> ParseRequestsFromBody(
287287
HttpRequest request,
288288
CancellationToken cancellationToken)
289-
=> RequestParser.ReadJsonRequestAsync(request.Body, cancellationToken);
289+
=> RequestParser.ParseRequestAsync(request.Body, cancellationToken);
290290

291291
private static bool TryParseOperations(
292292
string operationNameString,

‎src/HotChocolate/AspNetCore/src/AspNetCore/Serialization/DefaultHttpRequestParser.cs

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
using System.Buffers;
2+
using System.Runtime.CompilerServices;
3+
using System.Runtime.InteropServices;
24
using Microsoft.AspNetCore.Http;
35
using HotChocolate.Language;
46
using HotChocolate.Utilities;
@@ -38,12 +40,12 @@ public DefaultHttpRequestParser(
3840
throw new ArgumentNullException(nameof(parserOptions));
3941
}
4042

41-
public ValueTask<IReadOnlyList<GraphQLRequest>> ReadJsonRequestAsync(
43+
public ValueTask<IReadOnlyList<GraphQLRequest>> ParseRequestAsync(
4244
Stream stream,
4345
CancellationToken cancellationToken) =>
44-
ReadAsync(stream, false, cancellationToken);
46+
ReadAsync(stream, cancellationToken);
4547

46-
public GraphQLRequest ReadParamsRequest(IQueryCollection parameters)
48+
public GraphQLRequest ParseRequestFromParams(IQueryCollection parameters)
4749
{
4850
// next we deserialize the GET request with the query request builder ...
4951
string? query = parameters[QueryKey];
@@ -52,7 +54,7 @@ public GraphQLRequest ReadParamsRequest(IQueryCollection parameters)
5254
IReadOnlyDictionary<string, object?>? extensions = null;
5355

5456
// if we have no query or query id we cannot execute anything.
55-
if (string.IsNullOrEmpty(query) && string.IsNullOrEmpty(queryId))
57+
if (string.IsNullOrWhiteSpace(query) && string.IsNullOrWhiteSpace(queryId))
5658
{
5759
// so, if we do not find a top-level query or top-level id we will try to parse
5860
// the extensions and look in the extensions for Apollo`s active persisted
@@ -75,6 +77,11 @@ public GraphQLRequest ReadParamsRequest(IQueryCollection parameters)
7577
queryId = hash;
7678
}
7779

80+
if (!string.IsNullOrWhiteSpace(queryId))
81+
{
82+
EnsureValidQueryId(queryId);
83+
}
84+
7885
try
7986
{
8087
string? queryHash = null;
@@ -141,19 +148,17 @@ public GraphQLRequest ReadParamsRequest(IQueryCollection parameters)
141148
return (queryHash, document);
142149
}
143150

144-
public IReadOnlyList<GraphQLRequest> ReadOperationsRequest(
145-
string operations) =>
146-
Parse(operations, _parserOptions, _documentCache, _documentHashProvider);
151+
public IReadOnlyList<GraphQLRequest> ParseRequest(
152+
string operations)
153+
=> EnsureValidQueryId(Parse(operations, _parserOptions, _documentCache, _documentHashProvider));
147154

148155
private async ValueTask<IReadOnlyList<GraphQLRequest>> ReadAsync(
149156
Stream stream,
150-
bool isGraphQLQuery,
151157
CancellationToken cancellationToken)
152158
{
153159
try
154160
{
155-
Func<byte[], int, IReadOnlyList<GraphQLRequest>> parse =
156-
isGraphQLQuery ? ParseQuery : ParseRequest;
161+
Func<byte[], int, IReadOnlyList<GraphQLRequest>> parse = ParseRequest;
157162

158163
return await BufferHelper.ReadAsync(
159164
stream,
@@ -171,6 +176,10 @@ private async ValueTask<IReadOnlyList<GraphQLRequest>> ReadAsync(
171176
static () => throw DefaultHttpRequestParser_MaxRequestSizeExceeded(),
172177
cancellationToken);
173178
}
179+
catch (GraphQLRequestException)
180+
{
181+
throw;
182+
}
174183
catch (SyntaxException ex)
175184
{
176185
throw DefaultHttpRequestParser_SyntaxError(ex);
@@ -186,29 +195,67 @@ private IReadOnlyList<GraphQLRequest> ParseRequest(
186195
int bytesBuffered)
187196
{
188197
var graphQLData = new ReadOnlySpan<byte>(buffer);
189-
graphQLData = graphQLData.Slice(0, bytesBuffered);
198+
graphQLData = graphQLData[..bytesBuffered];
190199

191200
var requestParser = new Utf8GraphQLRequestParser(
192201
graphQLData,
193202
_parserOptions,
194203
_documentCache,
195204
_documentHashProvider);
205+
206+
return EnsureValidQueryId(requestParser.Parse());
207+
}
208+
209+
internal static IReadOnlyList<GraphQLRequest> EnsureValidQueryId(IReadOnlyList<GraphQLRequest> requests)
210+
{
211+
if (requests.Count == 1)
212+
{
213+
var request = requests[0];
214+
if (!string.IsNullOrWhiteSpace(request.QueryId))
215+
{
216+
EnsureValidQueryId(request.QueryId);
217+
}
218+
return requests;
219+
}
196220

197-
return requestParser.Parse();
221+
foreach (var request in requests)
222+
{
223+
if (!string.IsNullOrWhiteSpace(request.QueryId))
224+
{
225+
EnsureValidQueryId(request.QueryId);
226+
}
227+
}
228+
return requests;
198229
}
199230

200-
private IReadOnlyList<GraphQLRequest> ParseQuery(
201-
byte[] buffer,
202-
int bytesBuffered)
231+
private static void EnsureValidQueryId(string queryId)
203232
{
204-
var graphQLData = new ReadOnlySpan<byte>(buffer);
205-
graphQLData = graphQLData.Slice(0, bytesBuffered);
233+
var span = queryId.AsSpan();
234+
ref var start = ref MemoryMarshal.GetReference(span);
235+
ref var end = ref Unsafe.Add(ref start, span.Length);
206236

207-
var requestParser = new Utf8GraphQLParser(graphQLData, _parserOptions);
237+
while (Unsafe.IsAddressLessThan(ref start, ref end))
238+
{
239+
if (!IsLetterOrDigitOrUnderscoreOrHyphen((byte)start))
240+
{
241+
throw ErrorHelper.InvalidQueryIdFormat();
242+
}
243+
start = ref Unsafe.Add(ref start, 1)!;
244+
}
245+
}
208246

209-
var queryHash = _documentHashProvider.ComputeHash(graphQLData);
210-
var document = requestParser.Parse();
247+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
248+
private static bool IsLetterOrDigitOrUnderscoreOrHyphen(byte c)
249+
{
250+
switch (c)
251+
{
252+
case > 96 and < 123 or > 64 and < 91:
253+
case > 47 and < 58:
254+
case 45 or 95:
255+
return true;
211256

212-
return new[] { new GraphQLRequest(document, queryHash), };
257+
default:
258+
return false;
259+
}
213260
}
214261
}

‎src/HotChocolate/AspNetCore/src/AspNetCore/Serialization/IHttpRequestParser.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,29 +20,29 @@ public interface IHttpRequestParser
2020
/// <returns>
2121
/// Returns the parsed GraphQL request.
2222
/// </returns>
23-
ValueTask<IReadOnlyList<GraphQLRequest>> ReadJsonRequestAsync(
23+
ValueTask<IReadOnlyList<GraphQLRequest>> ParseRequestAsync(
2424
Stream requestBody,
2525
CancellationToken cancellationToken);
2626

2727
/// <summary>
28-
/// Parses a GraphQL HTTP GET request from the HTTP query parameters.
28+
/// Parses the operations string from an GraphQL HTTP MultiPart request.
2929
/// </summary>
30-
/// <param name="parameters">
31-
/// The HTTP query parameter collection.
30+
/// <param name="operations">
31+
/// The operations string.
3232
/// </param>
3333
/// <returns>
3434
/// Returns the parsed GraphQL request.
3535
/// </returns>
36-
GraphQLRequest ReadParamsRequest(IQueryCollection parameters);
37-
36+
IReadOnlyList<GraphQLRequest> ParseRequest(string operations);
37+
3838
/// <summary>
39-
/// Parses the operations string from an GraphQL HTTP MultiPart request.
39+
/// Parses a GraphQL HTTP GET request from the HTTP query parameters.
4040
/// </summary>
41-
/// <param name="operations">
42-
/// The operations string.
41+
/// <param name="parameters">
42+
/// The HTTP query parameter collection.
4343
/// </param>
4444
/// <returns>
4545
/// Returns the parsed GraphQL request.
4646
/// </returns>
47-
IReadOnlyList<GraphQLRequest> ReadOperationsRequest(string operations);
47+
GraphQLRequest ParseRequestFromParams(IQueryCollection parameters);
4848
}

‎src/HotChocolate/AspNetCore/src/AspNetCore/Subscriptions/Protocols/Apollo/ApolloSubscriptionProtocolHandler.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Buffers;
22
using System.Diagnostics.CodeAnalysis;
33
using System.Text.Json;
4+
using HotChocolate.AspNetCore.Serialization;
45
using HotChocolate.Language;
56
using HotChocolate.Execution.Serialization;
67
using HotChocolate.Utilities;
@@ -149,6 +150,25 @@ await connection.CloseAsync(
149150
return;
150151
}
151152
}
153+
catch (GraphQLRequestException ex)
154+
{
155+
if (!root.TryGetProperty(Id, out idProp) ||
156+
idProp.ValueKind is not JsonValueKind.String ||
157+
string.IsNullOrEmpty(idProp.GetString()))
158+
{
159+
await connection.CloseAsync(
160+
Apollo_OnReceive_InvalidMessageType,
161+
CloseReasons.InvalidMessage,
162+
cancellationToken);
163+
return;
164+
}
165+
166+
await SendErrorMessageAsync(
167+
session,
168+
idProp.GetString()!,
169+
ex.Errors,
170+
cancellationToken);
171+
}
152172
catch (SyntaxException ex)
153173
{
154174
if (!root.TryGetProperty(Id, out idProp) ||
@@ -325,7 +345,8 @@ idProp.ValueKind is not JsonValueKind.String ||
325345
message = null;
326346
return false;
327347
}
328-
348+
349+
DefaultHttpRequestParser.EnsureValidQueryId(request);
329350
message = new DataStartMessage(id, request[0]);
330351
return true;
331352
}

‎src/HotChocolate/AspNetCore/src/AspNetCore/Subscriptions/Protocols/GraphQLOverWebSocket/GraphQLOverWebSocketProtocolHandler.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Buffers;
22
using System.Diagnostics.CodeAnalysis;
33
using System.Text.Json;
4+
using HotChocolate.AspNetCore.Serialization;
45
using HotChocolate.Execution.Serialization;
56
using HotChocolate.Language;
67
using HotChocolate.Utilities;
@@ -147,6 +148,22 @@ await SendConnectionAcceptMessage(
147148
return;
148149
}
149150
}
151+
catch (GraphQLRequestException ex)
152+
{
153+
if (!root.TryGetProperty(Id, out idProp) ||
154+
idProp.ValueKind is not JsonValueKind.String ||
155+
string.IsNullOrEmpty(idProp.GetString()))
156+
{
157+
await connection.CloseInvalidSubscribeMessageAsync(cancellationToken);
158+
return;
159+
}
160+
161+
await SendErrorMessageAsync(
162+
session,
163+
idProp.GetString()!,
164+
ex.Errors,
165+
cancellationToken);
166+
}
150167
catch (SyntaxException ex)
151168
{
152169
if (!root.TryGetProperty(Id, out idProp) ||
@@ -315,6 +332,7 @@ idProp.ValueKind is not JsonValueKind.String ||
315332
return false;
316333
}
317334

335+
DefaultHttpRequestParser.EnsureValidQueryId(request);
318336
message = new SubscribeMessage(id, request[0]);
319337
return true;
320338
}

0 commit comments

Comments
 (0)