Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BadHttpRequestException: Unexpected end of request content. #238

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Fix BadHttpRequestException: Unexpected end of request content. #238

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d8a420
Fix BadHttpRequestException: Unexpected end of request content.
Oct 7, 2021
0f1b3f1
Update NewPlatform.Flexberry.ORM.ODataService/Middleware/RequestHeade…
bratchikov Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions NewPlatform.Flexberry.ORM.ODataService/Middleware/RequestHeadersHookMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ public RequestHeadersHookMiddleware(RequestDelegate next)
/// <returns>A task that can be awaited.</returns>
public async Task Invoke(HttpContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}

// Skip if request has been aborted.
if (context.RequestAborted.IsCancellationRequested)
NicholasNoise marked this conversation as resolved.
Show resolved Hide resolved
{
await _next.Invoke(context);
}
Comment on lines +51 to +55
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ошибка в логике обработки отмены запроса

Текущая реализация продолжает обработку запроса при его отмене (IsCancellationRequested == true). Это противоречит ожидаемому поведению - при отмене запроса обработка должна быть прекращена.

Предлагаемое исправление:

 // Skip if request has been aborted.
 if (context.RequestAborted.IsCancellationRequested)
 {
-    await _next.Invoke(context);
+    return;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Skip if request has been aborted.
if (context.RequestAborted.IsCancellationRequested)
{
await _next.Invoke(context);
}
// Skip if request has been aborted.
if (context.RequestAborted.IsCancellationRequested)
{
return;
}


var request = context.Request;

IEnumerable<string> mimeTypeDirectives = AcceptHeaderParser.MimeTypeDirectivesFromAcceptHeader(request.Headers[HeaderNames.Accept]);
Expand All @@ -58,30 +69,34 @@ public async Task Invoke(HttpContext context)
{
request.EnableBuffering();

// Leave the body open so the next middleware can read it.
string bodyString;
using (var reader = new System.IO.StreamReader(
request.Body,
encoding: Encoding.UTF8,
detectEncodingFromByteOrderMarks: false,
bufferSize: 1024,
leaveOpen: true))
{
bodyString = await reader.ReadToEndAsync();
request.Body.Position = 0;
}
string bodyString = await GetBodyString(request);

context.Items.Add(PropertyKeyRequestContent, bodyString);
}

/*
/// Исправление для Mono, взято из https://github.com/OData/odata.net/issues/165
/// Исправление для Mono, взято из https://github.com/OData/odata.net/issues/165 .
*/
if (!request.Headers.ContainsKey("Accept-Charset"))
request.Headers.Add("Accept-Charset", new[] { "utf-8" });
if (!request.Headers.ContainsKey(HeaderNames.AcceptCharset))
request.Headers.Add(HeaderNames.AcceptCharset, new[] { "utf-8" });

await _next.Invoke(context);
}

private static async Task<string> GetBodyString(HttpRequest request)
{
// Leave the body open so the next middleware can read it.
using var reader = new System.IO.StreamReader(
request.Body,
Encoding.UTF8,
false,
1024,
true);
string bodyString = await reader.ReadToEndAsync();
request.Body.Position = 0;

return bodyString;
}
Comment on lines +86 to +99
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Рекомендации по оптимизации чтения тела запроса

Текущая реализация работоспособна, но есть возможности для улучшения:

  1. Размер буфера 1024 байта может быть недостаточным для больших запросов
  2. Отсутствует защита от чрезмерного потребления памяти при очень больших запросах

Предлагаемые улучшения:

 private static async Task<string> GetBodyString(HttpRequest request)
 {
+    const int maxRequestSize = 10 * 1024 * 1024; // 10 MB
+    if (request.ContentLength > maxRequestSize)
+    {
+        throw new InvalidOperationException("Размер запроса превышает допустимый предел");
+    }
+
     // Leave the body open so the next middleware can read it.
     using var reader = new System.IO.StreamReader(
         request.Body,
         Encoding.UTF8,
         false,
-        1024,
+        4096, // Увеличенный размер буфера
         true);
     string bodyString = await reader.ReadToEndAsync();
     request.Body.Position = 0;

     return bodyString;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static async Task<string> GetBodyString(HttpRequest request)
{
// Leave the body open so the next middleware can read it.
using var reader = new System.IO.StreamReader(
request.Body,
Encoding.UTF8,
false,
1024,
true);
string bodyString = await reader.ReadToEndAsync();
request.Body.Position = 0;
return bodyString;
}
private static async Task<string> GetBodyString(HttpRequest request)
{
const int maxRequestSize = 10 * 1024 * 1024; // 10 MB
if (request.ContentLength > maxRequestSize)
{
throw new InvalidOperationException("Размер запроса превышает допустимый предел");
}
// Leave the body open so the next middleware can read it.
using var reader = new System.IO.StreamReader(
request.Body,
Encoding.UTF8,
false,
4096, // Увеличенный размер буфера
true);
string bodyString = await reader.ReadToEndAsync();
request.Body.Position = 0;
return bodyString;
}

}
}
#endif
Loading