Skip to content

Commit 165abe9

Browse files
michaelstaibmichaelstaib
authored
Fix issue that parsed query for persisted operations (#9523)
1 parent fe1d819 commit 165abe9

15 files changed

Lines changed: 511 additions & 278 deletions

File tree

‎src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/ExecutorSession.cs‎

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
#if !NET9_0_OR_GREATER
22
using System.Diagnostics.CodeAnalysis;
33
#endif
4+
using System.IO.Pipelines;
45
using System.Net;
56
using HotChocolate.AspNetCore.Formatters;
67
using HotChocolate.AspNetCore.Instrumentation;
78
using HotChocolate.AspNetCore.Parsers;
89
using HotChocolate.AspNetCore.Utilities;
910
using HotChocolate.Features;
1011
using HotChocolate.Language;
12+
using HotChocolate.PersistedOperations;
1113
using Microsoft.AspNetCore.Http;
1214
using Microsoft.Extensions.DependencyInjection;
1315

@@ -25,6 +27,8 @@ public sealed class ExecutorSession
2527
private readonly IHttpRequestParser _requestParser;
2628
private readonly IHttpResponseFormatter _responseFormatter;
2729
private readonly IServerDiagnosticEvents _diagnosticEvents;
30+
private readonly bool _skipDocumentBody;
31+
private readonly IError? _operationNotAllowedError;
2832

2933
public ExecutorSession(IRequestExecutor executor)
3034
{
@@ -37,14 +41,13 @@ public ExecutorSession(IRequestExecutor executor)
3741
_responseFormatter = executor.Schema.Services.GetRequiredService<IHttpResponseFormatter>();
3842
_requestParser = executor.Schema.Services.GetRequiredService<IHttpRequestParser>();
3943
_diagnosticEvents = executor.Schema.Services.GetRequiredService<IServerDiagnosticEvents>();
44+
var persistedOps = executor.Schema.Services.GetService<PersistedOperationOptions>();
45+
_skipDocumentBody = persistedOps is { OnlyAllowPersistedDocuments: true, AllowDocumentBody: false };
46+
_operationNotAllowedError = persistedOps?.OperationNotAllowedError;
4047
}
4148

4249
public ISocketSessionInterceptor SocketSessionInterceptor => _socketSessionInterceptor;
4350

44-
public IHttpResponseFormatter ResponseFormatter => _responseFormatter;
45-
46-
public IHttpRequestParser RequestParser => _requestParser;
47-
4851
public IServerDiagnosticEvents DiagnosticEvents => _diagnosticEvents;
4952

5053
public ulong Version => _executor.Version;
@@ -187,6 +190,63 @@ public async Task<IResponseStream> ExecuteBatchAsync(
187190
cancellationToken: context.RequestAborted);
188191
}
189192

193+
public async ValueTask<GraphQLRequest[]> ParseRequestAsync(
194+
PipeReader requestBody,
195+
CancellationToken cancellationToken)
196+
{
197+
var requests = await _requestParser.ParseRequestAsync(requestBody, _skipDocumentBody, cancellationToken);
198+
ThrowIfDocumentBodyNotAllowed(requests);
199+
return requests;
200+
}
201+
202+
public async ValueTask<GraphQLRequest> ParsePersistedOperationRequestAsync(
203+
string documentId,
204+
string? operationName,
205+
PipeReader requestBody,
206+
CancellationToken cancellationToken)
207+
{
208+
var request = await _requestParser.ParsePersistedOperationRequestAsync(
209+
documentId, operationName, requestBody, _skipDocumentBody, cancellationToken);
210+
ThrowIfDocumentBodyNotAllowed(request);
211+
return request;
212+
}
213+
214+
public GraphQLRequest ParseRequestFromParams(IQueryCollection parameters)
215+
{
216+
var request = _requestParser.ParseRequestFromParams(parameters, _skipDocumentBody);
217+
ThrowIfDocumentBodyNotAllowed(request);
218+
return request;
219+
}
220+
221+
public GraphQLRequest ParsePersistedOperationRequestFromParams(
222+
string operationId,
223+
string? operationName,
224+
IQueryCollection parameters)
225+
=> _requestParser.ParsePersistedOperationRequestFromParams(operationId, operationName, parameters);
226+
227+
public GraphQLRequest[] ParseRequest(string sourceText)
228+
{
229+
var requests = _requestParser.ParseRequest(sourceText, _skipDocumentBody);
230+
ThrowIfDocumentBodyNotAllowed(requests);
231+
return requests;
232+
}
233+
234+
private void ThrowIfDocumentBodyNotAllowed(GraphQLRequest request)
235+
{
236+
if (_skipDocumentBody && request.HasDocumentBody && request.DocumentId is null)
237+
{
238+
throw new GraphQLRequestException(_operationNotAllowedError!);
239+
}
240+
}
241+
242+
private void ThrowIfDocumentBodyNotAllowed(GraphQLRequest[] requests)
243+
{
244+
foreach (var request in requests)
245+
{
246+
ThrowIfDocumentBodyNotAllowed(request);
247+
}
248+
}
249+
190250
public ValueTask WriteResultAsync(
191251
HttpContext context,
192252
IExecutionResult result,

‎src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/HotChocolate.AspNetCore.Pipeline.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
<ProjectReference Include="..\Transport.Formatters\HotChocolate.Transport.Formatters.csproj" />
1616
<ProjectReference Include="..\Transport.Sockets\HotChocolate.Transport.Sockets.csproj" />
1717
<ProjectReference Include="..\..\..\Caching\src\Caching.Memory\HotChocolate.Caching.Memory.csproj" />
18+
<ProjectReference Include="..\..\..\PersistedOperations\src\PersistedOperations.Abstractions\HotChocolate.PersistedOperations.Abstractions.csproj" />
1819
</ItemGroup>
1920

2021
<ItemGroup>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ protected override async ValueTask<GraphQLRequest[]> ParseRequestsFromBodyAsync(
107107

108108
// Parse the string values of interest from the IFormCollection
109109
var multipartRequest = ParseMultipartRequest(form);
110-
var requests = session.RequestParser.ParseRequest(multipartRequest.Operations);
110+
var requests = session.ParseRequest(multipartRequest.Operations);
111111

112112
// we add the file lookup as a feature on the HttpContext and can grab it from
113113
// there and put it on the GraphQL request.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ protected virtual async ValueTask<GraphQLRequest[]> ParseRequestsFromBodyAsync(
253253
ExecutorSession session)
254254
{
255255
var requests =
256-
await session.RequestParser.ParseRequestAsync(
256+
await session.ParseRequestAsync(
257257
context.Request.BodyReader,
258258
context.RequestAborted);
259259

‎src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/Parsers/DefaultHttpRequestParser.cs‎

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ public DefaultHttpRequestParser(
4646

4747
public async ValueTask<GraphQLRequest[]> ParseRequestAsync(
4848
PipeReader requestBody,
49+
bool skipDocumentBody,
4950
CancellationToken cancellationToken)
5051
{
5152
try
@@ -78,7 +79,8 @@ public async ValueTask<GraphQLRequest[]> ParseRequestAsync(
7879
var requestParser = new Utf8GraphQLRequestParser(
7980
_parserOptions,
8081
_documentCache,
81-
_documentHashProvider);
82+
_documentHashProvider,
83+
skipDocumentBody);
8284

8385
var requests = requestParser.Parse(result.Buffer);
8486

@@ -105,6 +107,7 @@ public async ValueTask<GraphQLRequest> ParsePersistedOperationRequestAsync(
105107
string documentId,
106108
string? operationName,
107109
PipeReader requestBody,
110+
bool skipDocumentBody,
108111
CancellationToken cancellationToken)
109112
{
110113
if (!OperationDocumentId.TryParse(documentId, out var parsedDocumentId))
@@ -143,7 +146,8 @@ public async ValueTask<GraphQLRequest> ParsePersistedOperationRequestAsync(
143146
var requestParser = new Utf8GraphQLRequestParser(
144147
_parserOptions,
145148
_documentCache,
146-
_documentHashProvider);
149+
_documentHashProvider,
150+
skipDocumentBody);
147151

148152
var request = requestParser.ParsePersistedOperation(parsedDocumentId, operationName, result.Buffer);
149153

@@ -166,10 +170,24 @@ public async ValueTask<GraphQLRequest> ParsePersistedOperationRequestAsync(
166170
}
167171
}
168172

169-
public GraphQLRequest ParseRequestFromParams(IQueryCollection parameters)
173+
public GraphQLRequest ParseRequestFromParams(IQueryCollection parameters, bool skipDocumentBody = false)
170174
{
171175
// next, we deserialize the GET request with the query request builder ...
172-
string? query = parameters[QueryKey];
176+
var hasDocumentBody = false;
177+
string? query = null;
178+
179+
if (skipDocumentBody)
180+
{
181+
if (!string.IsNullOrWhiteSpace((string?)parameters[QueryKey]))
182+
{
183+
hasDocumentBody = true;
184+
}
185+
}
186+
else
187+
{
188+
query = parameters[QueryKey];
189+
}
190+
173191
string? queryId = parameters[QueryIdKey];
174192
string? operationName = parameters[OperationNameKey];
175193
string? onError = parameters[OnErrorKey];
@@ -189,9 +207,19 @@ public GraphQLRequest ParseRequestFromParams(IQueryCollection parameters)
189207
// we will use the request parser utils to extract the hash from the extensions.
190208
if (!TryExtractHash(extensions, _documentHashProvider, out var hash))
191209
{
192-
// if we cannot find any query hash in the extensions, or if the extensions are
193-
// null, we are unable to execute and will throw a request error.
194-
throw DefaultHttpRequestParser_QueryAndIdMissing();
210+
if (hasDocumentBody)
211+
{
212+
// The request had a query parameter, but we skipped it because we are
213+
// in strict trusted documents mode. Let it through so the execution
214+
// pipeline can reject it with HC0067.
215+
hash = null;
216+
}
217+
else
218+
{
219+
// if we cannot find any query hash in the extensions, or if the extensions are
220+
// null, we are unable to execute and will throw a request error.
221+
throw DefaultHttpRequestParser_QueryAndIdMissing();
222+
}
195223
}
196224

197225
// if we however found a query hash, we will use it as a query id and move on
@@ -237,7 +265,8 @@ public GraphQLRequest ParseRequestFromParams(IQueryCollection parameters)
237265
operationName,
238266
errorHandlingMode,
239267
variableSet,
240-
extensions);
268+
extensions,
269+
hasDocumentBody);
241270
}
242271
catch (SyntaxException ex)
243272
{
@@ -344,7 +373,7 @@ public GraphQLRequest ParsePersistedOperationRequestFromParams(
344373
$"Unknown 'onError' value '{onError}'. Allowed values are 'PROPAGATE' or 'NULL'.");
345374
}
346375

347-
public GraphQLRequest[] ParseRequest(string sourceText)
376+
public GraphQLRequest[] ParseRequest(string sourceText, bool skipDocumentBody = false)
348377
{
349378
byte[]? rented = null;
350379
var maxLength = s_utf8.GetMaxByteCount(sourceText.Length);
@@ -353,7 +382,12 @@ public GraphQLRequest[] ParseRequest(string sourceText)
353382
try
354383
{
355384
s_utf8.GetBytes(sourceText, span);
356-
return Parse(span, _parserOptions, _documentCache, _documentHashProvider);
385+
var requestParser = new Utf8GraphQLRequestParser(
386+
_parserOptions,
387+
_documentCache,
388+
_documentHashProvider,
389+
skipDocumentBody);
390+
return requestParser.Parse(span);
357391
}
358392
catch (InvalidGraphQLRequestException ex)
359393
{

‎src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/Parsers/IHttpRequestParser.cs‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ public interface IHttpRequestParser
1515
/// <param name="requestBody">
1616
/// A stream representing the HTTP request body.
1717
/// </param>
18+
/// <param name="skipDocumentBody">
19+
/// If <c>true</c>, the document body will be skipped during parsing.
20+
/// </param>
1821
/// <param name="cancellationToken">
1922
/// The request cancellation token.
2023
/// </param>
@@ -23,6 +26,7 @@ public interface IHttpRequestParser
2326
/// </returns>
2427
ValueTask<GraphQLRequest[]> ParseRequestAsync(
2528
PipeReader requestBody,
29+
bool skipDocumentBody,
2630
CancellationToken cancellationToken);
2731

2832
/// <summary>
@@ -37,6 +41,9 @@ ValueTask<GraphQLRequest[]> ParseRequestAsync(
3741
/// <param name="requestBody">
3842
/// A stream representing the HTTP request body.
3943
/// </param>
44+
/// <param name="skipDocumentBody">
45+
/// If <c>true</c>, the document body will be skipped during parsing.
46+
/// </param>
4047
/// <param name="cancellationToken">
4148
/// The request cancellation token.
4249
/// </param>
@@ -47,6 +54,7 @@ ValueTask<GraphQLRequest> ParsePersistedOperationRequestAsync(
4754
string documentId,
4855
string? operationName,
4956
PipeReader requestBody,
57+
bool skipDocumentBody,
5058
CancellationToken cancellationToken);
5159

5260
/// <summary>
@@ -55,21 +63,27 @@ ValueTask<GraphQLRequest> ParsePersistedOperationRequestAsync(
5563
/// <param name="sourceText">
5664
/// The operations string.
5765
/// </param>
66+
/// <param name="skipDocumentBody">
67+
/// If <c>true</c>, the document body will be skipped during parsing.
68+
/// </param>
5869
/// <returns>
5970
/// Returns the parsed GraphQL request.
6071
/// </returns>
61-
GraphQLRequest[] ParseRequest(string sourceText);
72+
GraphQLRequest[] ParseRequest(string sourceText, bool skipDocumentBody = false);
6273

6374
/// <summary>
6475
/// Parses a GraphQL HTTP GET request from the HTTP query parameters.
6576
/// </summary>
6677
/// <param name="parameters">
6778
/// The HTTP query parameter collection.
6879
/// </param>
80+
/// <param name="skipDocumentBody">
81+
/// If <c>true</c>, the document body will be skipped during parsing.
82+
/// </param>
6983
/// <returns>
7084
/// Returns the parsed GraphQL request.
7185
/// </returns>
72-
GraphQLRequest ParseRequestFromParams(IQueryCollection parameters);
86+
GraphQLRequest ParseRequestFromParams(IQueryCollection parameters, bool skipDocumentBody = false);
7387

7488
/// <summary>
7589
/// Parses the variables and extensions from the HTTP query parameters.

‎src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/Utilities/MiddlewareHelper.cs‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static ValidateAcceptContentTypeResult ValidateAcceptContentType(
2727
HttpStatusCode.BadRequest);
2828
}
2929

30-
var requestFlags = executorSession.ResponseFormatter.CreateRequestFlags(headerResult.AcceptMediaTypes);
30+
var requestFlags = executorSession.CreateRequestFlags(headerResult.AcceptMediaTypes);
3131

3232
// if the request defines accept header values of which we cannot handle any provided
3333
// media type, then we will fail the request with 406 Not Acceptable.
@@ -55,7 +55,7 @@ public static ParseRequestResult ParseRequestFromParams(
5555
{
5656
try
5757
{
58-
var request = executorSession.RequestParser.ParseRequestFromParams(context.Request.Query);
58+
var request = executorSession.ParseRequestFromParams(context.Request.Query);
5959
context.Response.RegisterForDispose(request);
6060
return new ParseRequestResult(request);
6161
}
@@ -88,7 +88,7 @@ public static ParseRequestResult ParseVariablesAndExtensionsFromParams(
8888
try
8989
{
9090
var request =
91-
executorSession.RequestParser.ParsePersistedOperationRequestFromParams(
91+
executorSession.ParsePersistedOperationRequestFromParams(
9292
operationId,
9393
operationName,
9494
context.Request.Query);
@@ -125,7 +125,7 @@ public static async Task<ParseRequestResult> ParseSingleRequestFromBodyAsync(
125125
try
126126
{
127127
request =
128-
await executorSession.RequestParser.ParsePersistedOperationRequestAsync(
128+
await executorSession.ParsePersistedOperationRequestAsync(
129129
operationId,
130130
operationName,
131131
context.Request.BodyReader,
@@ -286,12 +286,7 @@ public static async Task WriteResultAsync(
286286
formatScope = executorSession.DiagnosticEvents.FormatHttpResponse(context, queryResult);
287287
}
288288

289-
await executorSession.ResponseFormatter.FormatAsync(
290-
context.Response,
291-
result,
292-
acceptMediaTypes,
293-
statusCode,
294-
context.RequestAborted);
289+
await executorSession.WriteResultAsync(context, result, acceptMediaTypes, statusCode);
295290
}
296291
finally
297292
{

‎src/HotChocolate/AspNetCore/test/AspNetCore.Tests/PersistedOperationTests.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,11 @@ public async Task Standard_Query_Not_Allowed_Override_Per_Request()
401401
var server = CreateStarWarsServer(
402402
configureServices: s => s
403403
.AddGraphQL("StarWars")
404-
.ModifyRequestOptions(o => o.PersistedOperations.OnlyAllowPersistedDocuments = true)
404+
.ModifyRequestOptions(o =>
405+
{
406+
o.PersistedOperations.OnlyAllowPersistedDocuments = true;
407+
o.PersistedOperations.AllowDocumentBody = true;
408+
})
405409
.ConfigureSchemaServices(c => c.AddSingleton<IOperationDocumentStorage>(storage))
406410
.UsePersistedOperationPipeline()
407411
.AddHttpRequestInterceptor<AllowNonPersistedOperationInterceptor>());

0 commit comments

Comments
 (0)