Skip to content

Commit fc51c8c

Browse files
bobbyg603bobbyg603
authored
fix: large attachments (#81)
* fix: out-of-memory crash for large attachments * chore: better logging * chore: remove todo * chore: update BugSplatDotNetStandard/Utils/TempFile.cs
1 parent 8ae0228 commit fc51c8c

File tree

9 files changed

+305
-200
lines changed

9 files changed

+305
-200
lines changed

‎BugSplatDotNetStandard.Test/Api/CrashPostClient.cs‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
using System.Net.Http;
1616
using System.Net;
1717
using System.IO.Compression;
18+
using System.Security.Cryptography;
19+
using System.Text.RegularExpressions;
1820

1921
namespace Tests
2022
{
@@ -116,6 +118,26 @@ public async Task CrashPostClient_PostMinidump_ShouldReturn200()
116118
Assert.AreEqual(HttpStatusCode.OK, postResult.StatusCode);
117119
}
118120

121+
[Test]
122+
[Explicit]
123+
public void CrashPostClient_PostMinidump_Returns400IfFileIsTooBig()
124+
{
125+
var bugsplat = new BugSplat(database, application, version);
126+
var sut = new CrashPostClient(HttpClientFactory.Default, S3ClientFactory.Default);
127+
var largeFile = CreateLargeTempFile(1000 * 1024 * 1024); // 1 GB
128+
129+
Assert.ThrowsAsync<Exception>(async () =>
130+
{
131+
await sut.PostMinidump(
132+
database,
133+
application,
134+
version,
135+
largeFile,
136+
MinidumpPostOptions.Create(bugsplat)
137+
);
138+
}, "Failed to parse upload url: crash post size limit exceeded");
139+
}
140+
119141
[Test]
120142
public void CrashPostClient_PostException_ShouldNotThrowIfAttachmentLocked()
121143
{
@@ -192,7 +214,7 @@ public async Task CrashPostClient_PostCrashFile_PostCrashAndMetadata()
192214
stackTrace,
193215
ExceptionPostOptions.Create(bugsplat)
194216
);
195-
217+
196218
var postResponseContent = JObject.Parse(postResult.Content.ReadAsStringAsync().Result);
197219
var id = postResponseContent["crashId"].Value<int>();
198220
var crashDetails = await crashDetailsClient.GetCrashDetails(database, id);
@@ -277,5 +299,36 @@ private Mock<HttpMessageHandler> CreateMockHttpClientForExceptionPost(string cra
277299

278300
return handlerMock;
279301
}
302+
303+
private static FileInfo CreateLargeTempFile(long sizeInBytes, string fileNamePrefix = "LargeTestFile")
304+
{
305+
if (sizeInBytes < 0)
306+
{
307+
throw new ArgumentException("Size cannot be negative", nameof(sizeInBytes));
308+
}
309+
310+
string tempFilePath = Path.Combine(Path.GetTempPath(), $"{fileNamePrefix}_{Guid.NewGuid()}.tmp");
311+
const int bufferSize = 81920;
312+
byte[] buffer = new byte[bufferSize];
313+
314+
using (var fileStream = new FileStream(tempFilePath, FileMode.Create, FileAccess.Write, FileShare.None, bufferSize))
315+
{
316+
fileStream.SetLength(sizeInBytes);
317+
318+
long bytesWritten = 0;
319+
using (var rng = RandomNumberGenerator.Create())
320+
{
321+
while (bytesWritten < sizeInBytes)
322+
{
323+
int bytesToWrite = (int)Math.Min(bufferSize, sizeInBytes - bytesWritten);
324+
rng.GetBytes(buffer, 0, bytesToWrite);
325+
fileStream.Write(buffer, 0, bytesToWrite);
326+
bytesWritten += bytesToWrite;
327+
}
328+
}
329+
}
330+
331+
return new FileInfo(tempFilePath);
332+
}
280333
}
281334
}

‎BugSplatDotNetStandard.Test/Api/VersionsClient.cs‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,22 @@ public void VersionsClient_UploadSymbolsFile_ShouldDeleteZipFileAfterUpload()
8686
{
8787
var application = "my-net-crasher";
8888
var version = Guid.NewGuid().ToString();
89-
var zipFileFullName = "test.zip";
89+
var zipFileFullName = string.Empty;
9090
var symbolFileInfo = new FileInfo("Files/myConsoleCrasher.pdb");
9191
var mockApiClient = CreateMockBugSplatApiClient();
9292
var mockS3ClientFactory = FakeS3ClientFactory.CreateMockS3ClientFactory();
93-
var mockZipUtils = CreateMockZipUtils(zipFileFullName);
93+
var realTempFileFactory = new TempFileFactory();
94+
var mockTempFileFactory = new Mock<ITempFileFactory>();
95+
mockTempFileFactory
96+
.Setup(factory => factory.CreateTempZip(It.IsAny<IEnumerable<FileInfo>>()))
97+
.Returns((IEnumerable<FileInfo> files) =>
98+
{
99+
var result = realTempFileFactory.CreateTempZip(files);
100+
zipFileFullName = result.File.FullName;
101+
return result;
102+
});
94103
var sut = new VersionsClient(mockApiClient, mockS3ClientFactory);
95-
sut.ZipUtils = mockZipUtils;
104+
sut.TempFileFactory = mockTempFileFactory.Object;
96105

97106
var uploadResult = sut.UploadSymbolFile(
98107
database,
@@ -119,23 +128,5 @@ private IBugSplatApiClient CreateMockBugSplatApiClient()
119128
.ReturnsAsync(presignedUrlResponse);
120129
return mockApiClient.Object;
121130
}
122-
123-
private IZipUtils CreateMockZipUtils(string zipFileFullName)
124-
{
125-
if (File.Exists(zipFileFullName))
126-
{
127-
File.Delete(zipFileFullName);
128-
}
129-
130-
var zipFileInfo = new ZipUtils().CreateZipFile(zipFileFullName, new List<FileInfo>());
131-
var mockZipUtils = new Mock<IZipUtils>();
132-
mockZipUtils
133-
.Setup(z => z.CreateZipFileFullName(It.IsAny<string>()))
134-
.Returns(zipFileFullName);
135-
mockZipUtils
136-
.Setup(z => z.CreateZipFile(It.IsAny<string>(), It.IsAny<IEnumerable<FileInfo>>()))
137-
.Returns(zipFileInfo);
138-
return mockZipUtils.Object;
139-
}
140131
}
141132
}

‎BugSplatDotNetStandard.Test/Helpers/FakeS3ClientFactory.cs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,12 @@ public static IS3ClientFactory CreateMockS3ClientFactory()
2626
{
2727
StatusCode = HttpStatusCode.OK
2828
};
29-
var s3UploadFileBytesResponse = new HttpResponseMessage
30-
{
31-
StatusCode = HttpStatusCode.OK,
32-
};
33-
s3UploadFileBytesResponse.Headers.Add("ETag", "\"test\"");
29+
s3UploadFileStreamResponse.Headers.Add("ETag", "\"test\"");
3430

3531
var mockS3Client = new Mock<IS3Client>();
3632
mockS3Client
3733
.Setup(s => s.UploadFileStreamToPresignedURL(It.IsAny<Uri>(), It.IsAny<Stream>()))
3834
.ReturnsAsync(s3UploadFileStreamResponse);
39-
mockS3Client
40-
.Setup(s => s.UploadFileBytesToPresignedURL(It.IsAny<Uri>(), It.IsAny<byte[]>()))
41-
.ReturnsAsync(s3UploadFileBytesResponse);
4235

4336
return new FakeS3ClientFactory(mockS3Client.Object);
4437
}

‎BugSplatDotNetStandard/Api/CrashPostClient.cs‎

Lines changed: 72 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace BugSplatDotNetStandard.Api
1616
{
1717
internal class CrashPostClient : IDisposable
1818
{
19-
internal IZipUtils ZipUtils { get; set; } = new ZipUtils();
19+
public ITempFileFactory TempFileFactory { get; set; } = new TempFileFactory();
2020
private HttpClient httpClient;
2121
private IS3Client s3Client;
2222

@@ -41,15 +41,17 @@ public async Task<HttpResponseMessage> PostException(
4141
ExceptionPostOptions overridePostOptions = null
4242
)
4343
{
44-
var inMemoryExceptionFile = new InMemoryFile() { FileName = "Callstack.txt", Content = Encoding.UTF8.GetBytes(stackTrace) };
45-
return await PostInMemoryCrashFile(
46-
database,
47-
application,
48-
version,
49-
inMemoryExceptionFile,
50-
defaultPostOptions,
51-
overridePostOptions
52-
);
44+
using (var stackTraceTempFile = TempFileFactory.CreateFromBytes("Callstack.txt", Encoding.UTF8.GetBytes(stackTrace)))
45+
{
46+
return await PostCrashFile(
47+
database,
48+
application,
49+
version,
50+
stackTraceTempFile.File,
51+
defaultPostOptions,
52+
overridePostOptions
53+
);
54+
}
5355
}
5456

5557
public async Task<HttpResponseMessage> PostMinidump(
@@ -61,12 +63,11 @@ public async Task<HttpResponseMessage> PostMinidump(
6163
MinidumpPostOptions overridePostOptions = null
6264
)
6365
{
64-
var inMemoryDmpFile = TryCreateInMemoryFileFromFileInfo(minidumpFileInfo);
65-
return await PostInMemoryCrashFile(
66+
return await PostCrashFile(
6667
database,
6768
application,
6869
version,
69-
inMemoryDmpFile,
70+
minidumpFileInfo,
7071
defaultPostOptions,
7172
overridePostOptions
7273
);
@@ -81,12 +82,11 @@ public async Task<HttpResponseMessage> PostXmlReport(
8182
XmlPostOptions overridePostOptions = null
8283
)
8384
{
84-
var inMemoryXmlFile = TryCreateInMemoryFileFromFileInfo(xmlFileInfo);
85-
return await PostInMemoryCrashFile(
85+
return await PostCrashFile(
8686
database,
8787
application,
8888
version,
89-
inMemoryXmlFile,
89+
xmlFileInfo,
9090
defaultPostOptions,
9191
overridePostOptions
9292
);
@@ -101,56 +101,73 @@ public async Task<HttpResponseMessage> PostCrashFile(
101101
BugSplatPostOptions overridePostOptions = null
102102
)
103103
{
104-
var inMemoryCrashFile = TryCreateInMemoryFileFromFileInfo(crashFileInfo);
105-
return await PostInMemoryCrashFile(
106-
database,
107-
application,
108-
version,
109-
inMemoryCrashFile,
110-
defaultPostOptions,
111-
overridePostOptions
112-
);
104+
overridePostOptions = overridePostOptions ?? new MinidumpPostOptions();
105+
106+
var additionalFormDataTempFiles = new List<ITempFile>();
107+
108+
try
109+
{
110+
additionalFormDataTempFiles = CombineWithDuplicatesRemoved(defaultPostOptions.FormDataParams, overridePostOptions.FormDataParams, (param) => param.Name)
111+
.Where(file => !string.IsNullOrEmpty(file.FileName) && file.Content != null)
112+
.Select(param => TempFileFactory.TryCreateFromBytes(param.FileName, param.Content.ReadAsByteArrayAsync().Result))
113+
.Where(temp => temp != null && temp.File.Exists)
114+
.ToList();
115+
116+
var additionalFormDataFiles = additionalFormDataTempFiles.Select(temp => temp.File).ToList();
117+
var attachments = CombineWithDuplicatesRemoved(defaultPostOptions.Attachments, overridePostOptions.Attachments, (file) => file.FullName)
118+
.Where(file => file != null && file.Exists)
119+
.ToList();
120+
121+
attachments.AddRange(additionalFormDataFiles);
122+
123+
return await PostCrash(
124+
database,
125+
application,
126+
version,
127+
crashFileInfo,
128+
attachments,
129+
defaultPostOptions,
130+
overridePostOptions
131+
);
132+
}
133+
finally
134+
{
135+
foreach (var tempFile in additionalFormDataTempFiles)
136+
{
137+
tempFile?.Dispose();
138+
}
139+
}
113140
}
114141

115-
private async Task<HttpResponseMessage> PostInMemoryCrashFile(
142+
private async Task<HttpResponseMessage> PostCrash(
116143
string database,
117144
string application,
118145
string version,
119-
InMemoryFile crashFile,
146+
FileInfo crashFileInfo,
147+
IEnumerable<FileInfo> attachments,
120148
BugSplatPostOptions defaultPostOptions,
121149
BugSplatPostOptions overridePostOptions = null
122150
)
123151
{
124-
overridePostOptions = overridePostOptions ?? new MinidumpPostOptions();
125-
126-
var files = CombineWithDuplicatesRemoved(defaultPostOptions.Attachments, overridePostOptions.Attachments, (file) => file.FullName)
127-
.Select(attachment => TryCreateInMemoryFileFromFileInfo(attachment))
128-
.Where(file => file != null)
129-
.ToList();
152+
var files = new List<FileInfo> { crashFileInfo };
153+
files.AddRange(attachments);
130154

131-
var additionalFormDataFiles = CombineWithDuplicatesRemoved(defaultPostOptions.FormDataParams, overridePostOptions.FormDataParams, (param) => param.Name)
132-
.Where(file => !string.IsNullOrEmpty(file.FileName) && file.Content != null)
133-
.Select(file => new InMemoryFile() { FileName = file.FileName, Content = file.Content.ReadAsByteArrayAsync().Result })
134-
.ToList();
135-
136-
files.Add(crashFile);
137-
files.AddRange(additionalFormDataFiles);
138-
139-
var zipBytes = ZipUtils.CreateInMemoryZipFile(files);
155+
using (var tempZipFile = TempFileFactory.CreateTempZip(files))
140156
using (
141157
var crashUploadResponse = await GetCrashUploadUrl(
142158
database,
143159
application,
144160
version,
145-
zipBytes.Length
161+
tempZipFile.File.Length
146162
)
147163
)
148164
{
149165
ThrowIfHttpRequestFailed(crashUploadResponse);
150166

151167
var presignedUrl = await GetPresignedUrlFromResponse(crashUploadResponse);
152168

153-
using (var uploadFileResponse = await this.s3Client.UploadFileBytesToPresignedURL(presignedUrl, zipBytes))
169+
using (var zipFileStream = tempZipFile.CreateFileStream())
170+
using (var uploadFileResponse = await s3Client.UploadFileStreamToPresignedURL(presignedUrl, zipFileStream))
154171
{
155172
ThrowIfHttpRequestFailed(uploadFileResponse);
156173

@@ -252,7 +269,7 @@ private async Task<HttpResponseMessage> GetCrashUploadUrl(
252269
string database,
253270
string application,
254271
string version,
255-
int crashPostSize
272+
long crashPostSize
256273
)
257274
{
258275
var baseUrl = this.CreateBaseUrlFromDatabase(database);
@@ -271,31 +288,23 @@ private string GetETagFromResponseHeaders(HttpHeaders headers)
271288

272289
private async Task<Uri> GetPresignedUrlFromResponse(HttpResponseMessage response)
273290
{
274-
try
275-
{
276-
var json = await response.Content.ReadAsStringAsync();
291+
var json = await response.Content.ReadAsStringAsync();
277292

278-
var jsonObj = new JsonObject(json);
279-
var url = jsonObj.GetValue("url");
293+
var jsonObj = new JsonObject(json);
294+
var url = jsonObj.TryGetValue("url");
295+
var message = jsonObj.TryGetValue("message");
280296

281-
return new Uri(url);
282-
}
283-
catch (Exception ex)
297+
if (string.IsNullOrEmpty(url) && !string.IsNullOrEmpty(message))
284298
{
285-
throw new Exception("Failed to parse crash upload url", ex);
299+
throw new Exception($"Failed to parse upload url: {message}");
286300
}
287-
}
288301

289-
private InMemoryFile TryCreateInMemoryFileFromFileInfo(FileInfo fileInfo)
290-
{
291-
try
302+
if (string.IsNullOrEmpty(url))
292303
{
293-
return InMemoryFile.FromFileInfo(fileInfo);
294-
}
295-
catch
296-
{
297-
return null;
304+
throw new Exception("Failed to parse upload url");
298305
}
306+
307+
return new Uri(url);
299308
}
300309
}
301310
}

0 commit comments

Comments
 (0)