Skip to content

Commit 100ac93

Browse files
const-cloudinaryconst-cloudinary
authored
Fix race condition in UploadLarge with parallel upload
1 parent 257fa16 commit 100ac93

File tree

5 files changed

+108
-25
lines changed

5 files changed

+108
-25
lines changed

‎CloudinaryDotNet.IntegrationTests/IntegrationTestBase.cs‎

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public partial class IntegrationTestBase
2222
protected string m_testImagePath;
2323
protected string m_testUnicodeImagePath;
2424
protected string m_testLargeImagePath;
25+
protected string m_testLargeFilePath;
2526
protected string m_testVideoPath;
2627
protected string m_testPdfPath;
2728
protected string m_testIconPath;
@@ -39,6 +40,7 @@ public partial class IntegrationTestBase
3940
protected const string TEST_UNICODE_IMAGE_NAME = "TestüniNämeLögö";
4041
protected const string TEST_UNICODE_IMAGE = "TestüniNämeLögö.jpg";
4142
protected const string TEST_LARGEIMAGE = "TestLargeImage.jpg";
43+
protected const string TEST_LARGEFILE = "TestLargeFile.txt";
4244
protected const string TEST_PDF = "multipage.pdf";
4345
protected const string TEST_FAVICON = "favicon.ico";
4446

@@ -163,19 +165,21 @@ public static string GetTaggedRandomValue()
163165

164166
private void SaveTestResources(Assembly assembly)
165167
{
166-
string filePrefix = Path.GetDirectoryName(assembly.Location);
167-
string testName = GetType().Name;
168+
var filePrefix = Path.GetDirectoryName(assembly.Location) ?? "";
169+
var testName = GetType().Name;
168170

169171
m_testVideoPath = Path.Combine(filePrefix, testName, TEST_MOVIE);
170172
m_testImagePath = Path.Combine(filePrefix, testName, TEST_IMAGE);
171173
m_testUnicodeImagePath = Path.Combine(filePrefix, testName, TEST_UNICODE_IMAGE);
172174
m_testLargeImagePath = Path.Combine(filePrefix, testName, TEST_LARGEIMAGE);
175+
m_testLargeFilePath = Path.Combine(filePrefix, testName, TEST_LARGEFILE);
173176
m_testPdfPath = Path.Combine(filePrefix, testName, TEST_PDF);
174177
m_testIconPath = Path.Combine(filePrefix, testName, TEST_FAVICON);
175178

176179
SaveEmbeddedToDisk(assembly, TEST_IMAGE, m_testImagePath);
177180
SaveEmbeddedToDisk(assembly, TEST_IMAGE, m_testUnicodeImagePath);
178181
SaveEmbeddedToDisk(assembly, TEST_LARGEIMAGE, m_testLargeImagePath);
182+
PopulateLargeFile(m_testLargeFilePath, 20971520);
179183
SaveEmbeddedToDisk(assembly, TEST_MOVIE, m_testVideoPath);
180184
SaveEmbeddedToDisk(assembly, TEST_FAVICON, m_testIconPath);
181185
SaveEmbeddedToDisk(assembly, TEST_PDF, m_testPdfPath);
@@ -195,20 +199,66 @@ private static void SaveEmbeddedToDisk(Assembly assembly, string sourcePath, str
195199
Directory.CreateDirectory(directoryName);
196200
}
197201

198-
Stream stream = assembly.GetManifestResourceStream(resName);
199-
using (FileStream fileStream = new FileStream(destPath, FileMode.CreateNew))
202+
var stream = assembly.GetManifestResourceStream(resName);
203+
using (var fileStream = new FileStream(destPath, FileMode.CreateNew))
200204
{
201205
stream.CopyTo(fileStream);
202206
}
203207
}
204208
catch
205209
{
210+
// ignored
211+
}
212+
}
206213

214+
private static void PopulateLargeFile(string filePath, long size, int chunkSize = 4096)
215+
{
216+
if (File.Exists(filePath))
217+
{
218+
return;
219+
}
220+
221+
// Write the initial binary data
222+
var initialData = new byte[]
223+
{
224+
0x42, 0x4D, 0x4A, 0xB9, 0x59, 0x00, 0x00, 0x00, 0x00, 0x00, 0x8A, 0x00, 0x00, 0x00, 0x7C, 0x00,
225+
0x00, 0x00, 0x78, 0x05, 0x00, 0x00, 0x78, 0x05, 0x00, 0x00, 0x01, 0x00, 0x18, 0x00, 0x00, 0x00,
226+
0x00, 0x00, 0xC0, 0xB8, 0x59, 0x00, 0x61, 0x0F, 0x00, 0x00, 0x61, 0x0F, 0x00, 0x00, 0x00, 0x00,
227+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0x00, 0xFF, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00,
228+
0x00, 0x00, 0x00, 0xFF, 0x42, 0x47, 0x52, 0x73, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
229+
0x54, 0xB8, 0x1E, 0xFC, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x66, 0x66, 0x66, 0xFC,
230+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xC4, 0xF5, 0x28, 0xFF, 0x00, 0x00, 0x00, 0x00,
231+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
232+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
233+
};
234+
235+
using (var fileStream = new FileStream(filePath, FileMode.CreateNew))
236+
{
237+
fileStream.Write(initialData, 0, initialData.Length);
238+
239+
// Calculate the remaining size to fill
240+
var remainingSize = size - fileStream.Length;
241+
242+
var buffer = new byte[chunkSize];
243+
244+
// Fill the buffer with 0xFF
245+
for (var i = 0; i < buffer.Length; i++)
246+
{
247+
buffer[i] = 0xFF;
248+
}
249+
250+
// Write chunks until the file reaches the specified size
251+
while (remainingSize > 0)
252+
{
253+
var currChunkSize = (int)Math.Min(remainingSize, chunkSize);
254+
fileStream.Write(buffer, 0, currChunkSize);
255+
remainingSize -= currChunkSize;
256+
}
207257
}
208258
}
209259

210260

211-
protected List<string> SplitFile(string sourceFile, int chunkSize, string suffix = "")
261+
protected static List<string> SplitFile(string sourceFile, int chunkSize, string suffix = "")
212262
{
213263
var chunks = new List<string>();
214264

@@ -520,6 +570,11 @@ protected static string GetFileMd5Sum(string filename)
520570
}
521571
}
522572

573+
protected static int GetFileSize(string filename)
574+
{
575+
return (int)new FileInfo(filename).Length;
576+
}
577+
523578
[OneTimeTearDown]
524579
public virtual void Cleanup()
525580
{

‎CloudinaryDotNet.IntegrationTests/UploadApi/UploadMethodsTest.cs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ public class UploadMethodsTest : IntegrationTestBase
4040

4141
protected string m_implicitTransformationText;
4242

43-
protected string m_largeFileEtag;
44-
protected int m_largeFileLength;
43+
protected string m_largeImageEtag;
44+
protected int m_largeImageLength;
4545

4646
public override void Initialize()
4747
{
@@ -56,8 +56,8 @@ public override void Initialize()
5656
m_transformationAr30, m_transformationAr12,
5757
m_eagerTransformation);
5858

59-
m_largeFileEtag = GetFileMd5Sum(m_testLargeImagePath);
60-
m_largeFileLength = (int)new FileInfo(m_testLargeImagePath).Length;
59+
m_largeImageEtag = GetFileMd5Sum(m_testLargeImagePath);
60+
m_largeImageLength = GetFileSize(m_testLargeImagePath);
6161
}
6262

6363
[Test, RetryWithDelay]
@@ -607,11 +607,21 @@ public async Task TestUploadLargeRawFilesAsyncInParallel()
607607

608608
var uploadParams = GetUploadLargeRawParams(largeFilePath);
609609

610-
var result = await m_cloudinary.UploadLargeAsync<RawUploadResult>(uploadParams, TEST_CHUNK_SIZE, 2);
610+
var result = await m_cloudinary.UploadLargeAsync<RawUploadResult>(uploadParams, TEST_CHUNK_SIZE, 4);
611611

612612
AssertUploadLarge(result);
613613
}
614614

615+
[Test, RetryWithDelay]
616+
public async Task TestUploadLargeRawFileEvenChunksAsyncInParallel()
617+
{
618+
var uploadParams = GetUploadLargeRawParams(m_testLargeFilePath);
619+
620+
var result = await m_cloudinary.UploadLargeAsync<RawUploadResult>(uploadParams, TEST_CHUNK_SIZE, 4);
621+
622+
AssertUploadLarge(result, GetFileSize(m_testLargeFilePath),GetFileMd5Sum(m_testLargeFilePath));
623+
}
624+
615625
private RawUploadParams GetUploadLargeRawParams(string path)
616626
{
617627
return new RawUploadParams()
@@ -625,11 +635,11 @@ private void AssertUploadLarge(RawUploadResult result, int fileLength = 0, strin
625635
{
626636
if (fileLength == 0)
627637
{
628-
fileLength = m_largeFileLength;
638+
fileLength = m_largeImageLength;
629639
}
630640
if (etag == null)
631641
{
632-
etag = m_largeFileEtag;
642+
etag = m_largeImageEtag;
633643
}
634644
Assert.NotNull(result);
635645
Assert.AreEqual(fileLength, result.Bytes, result.Error?.Message);

‎CloudinaryDotNet/Cloudinary.UploadApi.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,10 @@ public async Task<T> UploadLargeAsync<T>(
343343
}
344344

345345
parameters.File.Reset(bufferSize != 0 ? bufferSize : DEFAULT_CHUNK_SIZE);
346+
if (string.IsNullOrEmpty(parameters.UniqueUploadId))
347+
{
348+
parameters.UniqueUploadId = Utils.RandomPublicId();
349+
}
346350

347351
T result = null;
348352

‎CloudinaryDotNet/Core/LimitedStream.cs‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ namespace CloudinaryDotNet.Core
99
/// </summary>
1010
internal class LimitedStream : Stream
1111
{
12+
private readonly object streamLock;
1213
private readonly Stream originalStream;
1314
private long remainingBytes;
1415
private long startOffset;
@@ -21,11 +22,13 @@ internal class LimitedStream : Stream
2122
/// <param name="offset">The offset from which to start reading in the underlying stream.
2223
/// We ignore it for non-seekable streams.</param>
2324
/// <param name="maxBytes">Maximum bytes to read.</param>
24-
public LimitedStream(Stream stream, long offset, long maxBytes)
25+
/// <param name="streamLockObj">Stream lock object.</param>
26+
public LimitedStream(Stream stream, long offset, long maxBytes, object streamLockObj = null)
2527
{
2628
originalStream = stream ?? throw new ArgumentNullException(nameof(stream));
2729
remainingBytes = maxBytes;
2830
startOffset = currOffset = offset;
31+
streamLock = streamLockObj;
2932

3033
if (!stream.CanSeek)
3134
{
@@ -64,15 +67,16 @@ public override long Position
6467
/// <inheritdoc/>
6568
public override int Read(byte[] buffer, int offset, int count)
6669
{
67-
// make sure stream is not moved around.
68-
originalStream.Seek(currOffset, SeekOrigin.Begin);
69-
70-
var bytesRead = originalStream.Read(buffer, offset, (int)Math.Min(count, remainingBytes));
71-
72-
currOffset += bytesRead;
73-
remainingBytes -= bytesRead;
70+
lock (streamLock)
71+
{
72+
// make sure stream is not moved around.
73+
originalStream.Seek(currOffset, SeekOrigin.Begin);
74+
var bytesRead = originalStream.Read(buffer, offset, (int)Math.Min(count, remainingBytes));
75+
currOffset += bytesRead;
76+
remainingBytes -= bytesRead;
7477

75-
return bytesRead;
78+
return bytesRead;
79+
}
7680
}
7781

7882
/// <inheritdoc/>

‎CloudinaryDotNet/FileDescription.cs‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ public class FileDescription : IDisposable
4848

4949
private readonly object chunkLock = new ();
5050

51+
private readonly object streamLock = new ();
52+
5153
private Stream fileStream;
5254

5355
private string filePath;
@@ -148,7 +150,15 @@ public string FilePath
148150
{
149151
filePath = value;
150152
IsRemote = Utils.IsRemoteFile(filePath);
151-
FileName = IsRemote ? filePath : Path.GetFileName(filePath);
153+
if (IsRemote)
154+
{
155+
FileName = filePath;
156+
}
157+
else
158+
{
159+
FileName = Path.GetFileName(filePath);
160+
FileSize = GetFileLength();
161+
}
152162

153163
Reset();
154164
}
@@ -258,7 +268,7 @@ public void AddChunk(Stream chunkStream, long startByte, long chunkSize, bool la
258268
FileSize = CurrPos;
259269
}
260270

261-
var limitedStream = new LimitedStream(chunkStream, 0, chunkSize);
271+
var limitedStream = new LimitedStream(chunkStream, 0, chunkSize, streamLock);
262272
var chunk = new ChunkData(limitedStream, startByte, CurrPos - 1, FileSize)
263273
{
264274
LastChunk = last,
@@ -296,8 +306,8 @@ public async Task<ChunkData> GetNextChunkAsync(CancellationToken? cancellationTo
296306
{
297307
// Create a limited stream over the stream and return it.
298308
CurrChunkSize = Math.Min(BufferSize, chunkStream.Length - CurrPos);
299-
resultingStream = new LimitedStream(chunkStream, CurrPos, CurrChunkSize);
300-
if (CurrChunkSize < BufferSize && BufferSize != UnlimitedBuffer)
309+
resultingStream = new LimitedStream(chunkStream, CurrPos, CurrChunkSize, streamLock);
310+
if ((CurrChunkSize < BufferSize && BufferSize != UnlimitedBuffer) || CurrPos + CurrChunkSize == FileSize)
301311
{
302312
LastChunk = true;
303313
}

0 commit comments

Comments
 (0)