Skip to content

Commit e82b5b0

Browse files
srijkenniemyjski
authored andcommitted
POST data can get lost (#190)
* set input stream back to original position & don't read when that's impossible * add logging around POST processing * remove empty lines * Fix build * set leaveOpen to true in StreamReader" * store value instead of casting to string" * extract method GetPostData to reduce nesting" * add comment describing why leaveOpen is there * Process some review items * read body when content-length header is missing and limit reading * reduce property accesses * remove some blank lines * less property accesses and smaller buffer when possible
1 parent 676df78 commit e82b5b0

File tree

2 files changed

+149
-44
lines changed

2 files changed

+149
-44
lines changed

src/Platforms/Exceptionless.AspNetCore/RequestInfoCollector.cs

Lines changed: 74 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@
88
using Microsoft.Net.Http.Headers;
99
using Exceptionless.Models.Data;
1010
using Exceptionless.Extensions;
11+
using Exceptionless.Dependency;
12+
using System.Text;
1113

1214
namespace Exceptionless.AspNetCore {
1315
internal static class RequestInfoCollector {
16+
private const int MAX_BODY_SIZE = 50 * 1024;
1417
public static RequestInfo Collect(HttpContext context, ExceptionlessConfiguration config) {
1518
if (context == null)
1619
return null;
@@ -44,33 +47,82 @@ public static RequestInfo Collect(HttpContext context, ExceptionlessConfiguratio
4447
info.QueryString = context.Request.Query.ToDictionary(exclusionList);
4548

4649
if (config.IncludePostData) {
47-
if (context.Request.HasFormContentType && context.Request.Form.Count > 0) {
48-
info.PostData = context.Request.Form.ToDictionary(exclusionList);
49-
} else if (context.Request.ContentLength.HasValue && context.Request.ContentLength.Value > 0) {
50-
if (context.Request.ContentLength.Value < 1024 * 50) {
51-
try {
52-
if (context.Request.Body.CanSeek && context.Request.Body.Position > 0)
53-
context.Request.Body.Position = 0;
54-
55-
if (context.Request.Body.Position == 0) {
56-
using (var inputStream = new StreamReader(context.Request.Body))
57-
info.PostData = inputStream.ReadToEnd();
58-
} else {
59-
info.PostData = "Unable to get POST data: The stream could not be reset.";
60-
}
61-
} catch (Exception ex) {
62-
info.PostData = "Error retrieving POST data: " + ex.Message;
63-
}
64-
} else {
65-
string value = Math.Round(context.Request.ContentLength.Value / 1024m, 0).ToString("N0");
66-
info.PostData = String.Format("Data is too large ({0}kb) to be included.", value);
67-
}
68-
}
50+
info.PostData = GetPostData(context, config, exclusionList);
6951
}
7052

7153
return info;
7254
}
7355

56+
private static object GetPostData(HttpContext context, ExceptionlessConfiguration config, string[] exclusionList) {
57+
var log = config.Resolver.GetLog();
58+
59+
if (context.Request.HasFormContentType && context.Request.Form.Count > 0) {
60+
log.Debug("Reading POST data from Request.Form");
61+
return context.Request.Form.ToDictionary(exclusionList);
62+
}
63+
64+
var contentLength = context.Request.ContentLength.GetValueOrDefault();
65+
if(contentLength == 0) {
66+
string message = "Content-length was zero, empty post.";
67+
log.Debug(message);
68+
return message;
69+
}
70+
71+
if (contentLength > MAX_BODY_SIZE) {
72+
string value = Math.Round(contentLength / 1024m, 0).ToString("N0");
73+
string message = String.Format("Data is too large ({0}kb) to be included.", value);
74+
log.Debug(message);
75+
return message;
76+
}
77+
78+
try {
79+
if (!context.Request.Body.CanSeek) {
80+
string message = "Unable to get POST data: The stream could not be reset.";
81+
log.Debug(message);
82+
return message;
83+
}
84+
85+
long originalPosition = context.Request.Body.Position;
86+
if (originalPosition > 0) {
87+
context.Request.Body.Position = 0;
88+
}
89+
90+
log.Debug($"Reading POST, original position: {originalPosition}");
91+
92+
if (context.Request.Body.Position != 0) {
93+
string message = "Unable to get POST data: The stream position was not at 0.";
94+
log.Debug(message);
95+
return message;
96+
}
97+
98+
var maxDataToRead = contentLength == 0 ? MAX_BODY_SIZE : contentLength;
99+
100+
// pass default values, except for leaveOpen: true. This prevents us from disposing the underlying stream
101+
using (var inputStream = new StreamReader(context.Request.Body, Encoding.UTF8, true, 1024, true)) {
102+
var sb = new StringBuilder();
103+
int numRead;
104+
105+
int bufferSize = (int)Math.Min(1024, maxDataToRead);
106+
107+
char[] buffer = new char[bufferSize];
108+
while ((numRead = inputStream.ReadBlock(buffer, 0, bufferSize)) > 0 && (sb.Length + numRead) < maxDataToRead) {
109+
sb.Append(buffer, 0, numRead);
110+
}
111+
string postData = sb.ToString();
112+
113+
context.Request.Body.Position = originalPosition;
114+
115+
log.Debug($"Reading POST, set back to position: {originalPosition}");
116+
return postData;
117+
}
118+
}
119+
catch (Exception ex) {
120+
string message = $"Error retrieving POST data: {ex.Message}";
121+
log.Error(message);
122+
return message;
123+
}
124+
}
125+
74126
private static readonly List<string> _ignoredFormFields = new List<string> {
75127
"__*"
76128
};

src/Platforms/Exceptionless.Web/RequestInfoCollector.cs

Lines changed: 75 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Collections.Specialized;
4+
using System.Diagnostics;
45
using System.IO;
56
using System.Linq;
7+
using System.Text;
68
using System.Web;
79
using Exceptionless.Dependency;
810
using Exceptionless.Extensions;
@@ -12,6 +14,7 @@
1214
namespace Exceptionless.ExtendedData {
1315
internal static class RequestInfoCollector {
1416
private const int MAX_DATA_ITEM_LENGTH = 1000;
17+
private const int MAX_BODY_SIZE = 50*1024;
1518

1619
public static RequestInfo Collect(HttpContextBase context, ExceptionlessConfiguration config) {
1720
if (context == null)
@@ -52,28 +55,7 @@ public static RequestInfo Collect(HttpContextBase context, ExceptionlessConfigur
5255
info.Cookies = context.Request.Cookies.ToDictionary(exclusionList);
5356

5457
if (config.IncludePostData) {
55-
if (context.Request.Form.Count > 0) {
56-
info.PostData = context.Request.Form.ToDictionary(exclusionList);
57-
} else if (context.Request.ContentLength > 0) {
58-
if (context.Request.ContentLength < 1024 * 50) {
59-
try {
60-
if (context.Request.InputStream.CanSeek && context.Request.InputStream.Position > 0)
61-
context.Request.InputStream.Position = 0;
62-
63-
if (context.Request.InputStream.Position == 0) {
64-
using (var inputStream = new StreamReader(context.Request.InputStream))
65-
info.PostData = inputStream.ReadToEnd();
66-
} else {
67-
info.PostData = "Unable to get POST data: The stream could not be reset.";
68-
}
69-
} catch (Exception ex) {
70-
info.PostData = "Error retrieving POST data: " + ex.Message;
71-
}
72-
} else {
73-
string value = Math.Round(context.Request.ContentLength / 1024m, 0).ToString("N0");
74-
info.PostData = String.Format("Data is too large ({0}kb) to be included.", value);
75-
}
76-
}
58+
info.PostData = GetPostData(context, config, exclusionList);
7759
}
7860

7961
if (config.IncludeQueryString) {
@@ -87,6 +69,77 @@ public static RequestInfo Collect(HttpContextBase context, ExceptionlessConfigur
8769
return info;
8870
}
8971

72+
private static object GetPostData(HttpContextBase context, ExceptionlessConfiguration config, string[] exclusionList) {
73+
var log = config.Resolver.GetLog();
74+
75+
if (context.Request.Form.Count > 0) {
76+
log.Debug("Reading POST data from Request.Form");
77+
78+
return context.Request.Form.ToDictionary(exclusionList);
79+
}
80+
81+
var contentLength = context.Request.ContentLength;
82+
if (contentLength == 0) {
83+
string message = "Content-length was zero, empty post.";
84+
log.Debug(message);
85+
return message;
86+
}
87+
88+
if (contentLength > MAX_BODY_SIZE) {
89+
string value = Math.Round(contentLength / 1024m, 0).ToString("N0");
90+
string message = String.Format("Data is too large ({0}kb) to be included.", value);
91+
log.Debug(message);
92+
return message;
93+
}
94+
95+
try {
96+
if (!context.Request.InputStream.CanSeek) {
97+
string message = "Unable to get POST data: The stream could not be reset.";
98+
log.Debug(message);
99+
return message;
100+
}
101+
102+
long originalPosition = context.Request.InputStream.Position;
103+
if (context.Request.InputStream.Position > 0) {
104+
context.Request.InputStream.Position = 0;
105+
}
106+
107+
log.FormattedDebug("Reading POST, original position: {0}", originalPosition);
108+
109+
if (context.Request.InputStream.Position != 0) {
110+
string message = "Unable to get POST data: The stream position was not at 0.";
111+
log.Debug(message);
112+
return message;
113+
}
114+
115+
var maxDataToRead = contentLength == 0 ? MAX_BODY_SIZE : contentLength;
116+
117+
// pass default values, except for leaveOpen: true. This prevents us from disposing the underlying stream
118+
using (var inputStream = new StreamReader(context.Request.InputStream, Encoding.UTF8, true, 1024, true)) {
119+
var sb = new StringBuilder();
120+
int numRead;
121+
122+
int bufferSize = Math.Min(1024, maxDataToRead);
123+
124+
char[] buffer = new char[bufferSize];
125+
while ((numRead = inputStream.ReadBlock(buffer, 0, bufferSize)) > 0 && (sb.Length + numRead) < maxDataToRead) {
126+
sb.Append(buffer, 0, numRead);
127+
}
128+
string postData = sb.ToString();
129+
130+
context.Request.InputStream.Position = originalPosition;
131+
132+
log.FormattedDebug("Reading POST, set back to position: {0}", originalPosition);
133+
return postData;
134+
}
135+
}
136+
catch (Exception ex) {
137+
string message = $"Error retrieving POST data: {ex.Message}";
138+
log.Error(message);
139+
return message;
140+
}
141+
}
142+
90143
private static readonly List<string> _ignoredFormFields = new List<string> {
91144
"__*"
92145
};

0 commit comments

Comments
 (0)