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.

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

Issue 92 #95

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
Next Next commit
Add TargetFolder filed, rename BlobName to RenameToBlobName and BlobF…
…olderName to RenameToFolderName, modifed UploadBlob method and existing tests to cover the TargetFolder field.
  • Loading branch information
MichalFrends1 committed Nov 25, 2024
commit 86d45be8f66fa05bc608eb908c4a3a49380af62e
Original file line number Diff line number Diff line change
@@ -63,6 +63,7 @@ public void TestSetup()
PageOffset = default,
ParallelOperations = default,
ResizeFile = default,
TargetFolder = "targetfolder",
};

_destinationOA = new Destination
@@ -83,6 +84,7 @@ public void TestSetup()
PageOffset = default,
ParallelOperations = default,
ResizeFile = default,
TargetFolder = "targetfolder",
};

_options = new Options() { ThrowErrorOnFailure = true };
@@ -104,6 +106,8 @@ public async Task UploadFile_WithAndWithoutTags()

var _sourceWithoutTags = _source;
_sourceWithoutTags.Tags = null;
_destinationCS.TargetFolder = null;
_destinationOA.TargetFolder = null;

foreach (var blobtype in _blobtypes)
{
@@ -137,11 +141,13 @@ public async Task UploadFile_WithAndWithoutTags()
[TestMethod]
public async Task UploadFile_SetBlobName_ForceEncoding_ForceContentType()
{
_source.BlobName = "SomeBlob";
_source.RenameToBlobName = "SomeBlob";
_destinationCS.FileEncoding = "foo/bar";
_destinationCS.ContentType = "text/xml";
_destinationCS.TargetFolder = null;
_destinationOA.FileEncoding = "foo/bar";
_destinationOA.ContentType = "text/xml";
_destinationOA.TargetFolder = null;

var container = GetBlobContainer(_connectionString, _containerName);

@@ -167,7 +173,7 @@ public async Task UploadFile_SetBlobName_ForceEncoding_ForceContentType()
[TestMethod]
public async Task UploadFile_Compress()
{
_source.BlobName = "compress.gz";
_source.RenameToBlobName = "compress.gz";
_source.Compress = true;

var container = GetBlobContainer(_connectionString, _containerName);
@@ -180,14 +186,14 @@ public async Task UploadFile_Compress()
// connection string
var result = await AzureBlobStorage.UploadBlob(_source, _destinationCS, _options, default);
Assert.IsTrue(result.Success);
Assert.IsTrue(result.Data.ContainsValue($"{container.Uri}/compress.gz"));
Assert.IsTrue(await container.GetBlobClient("compress.gz").ExistsAsync(), "Uploaded SomeBlob blob should exist");
Assert.IsTrue(result.Data.ContainsValue($"{container.Uri}/targetfolder/compress.gz"));
Assert.IsTrue(await container.GetBlobClient("targetfolder/compress.gz").ExistsAsync(), "Uploaded SomeBlob blob should exist");

// OAuth
var result2 = await AzureBlobStorage.UploadBlob(_source, _destinationOA, _options, default);
Assert.IsTrue(result2.Success);
Assert.IsTrue(result2.Data.ContainsValue($"{container.Uri}/compress.gz"));
Assert.IsTrue(await container.GetBlobClient("compress.gz").ExistsAsync(), "Uploaded SomeBlob blob should exist");
Assert.IsTrue(result2.Data.ContainsValue($"{container.Uri}/targetfolder/compress.gz"));
Assert.IsTrue(await container.GetBlobClient("targetfolder/compress.gz").ExistsAsync(), "Uploaded SomeBlob blob should exist");
}
}

@@ -196,7 +202,7 @@ public async Task UploadFile_HandleExistingFile()
{
var errorHandlers = new List<HandleExistingFile>() { HandleExistingFile.Append, HandleExistingFile.Overwrite, HandleExistingFile.Error };
var _source2 = _source;
_source2.BlobName = "testfile.txt";
_source2.RenameToBlobName = "testfile.txt";
_source2.SourceFile = _testfile2;

foreach (var blobtype in _blobtypes)
@@ -217,23 +223,23 @@ public async Task UploadFile_HandleExistingFile()
var container = GetBlobContainer(_connectionString, _containerName);
var result = await AzureBlobStorage.UploadBlob(_source, _destinationCS, _options, default);
Assert.IsTrue(result.Success);
Assert.IsTrue(result.Data.ContainsValue($"{container.Uri}/testfile.txt"));
Assert.IsTrue(await container.GetBlobClient("testfile.txt").ExistsAsync(), "Uploaded testfile.txt blob should exist");
Assert.IsTrue(result.Data.ContainsValue($"{container.Uri}/targetfolder/testfile.txt"));
Assert.IsTrue(await container.GetBlobClient("targetfolder/testfile.txt").ExistsAsync(), "Uploaded targetfolder/testfile.txt blob should exist");

if (handler is HandleExistingFile.Append)
{
var result2 = await AzureBlobStorage.UploadBlob(_source2, _destinationCS, _options, default);
// You can use Azure Portal to check if the blob contains 2x "Etiam dui".
Assert.IsTrue(result2.Success);
Assert.IsTrue(result2.Data.ContainsValue($"{container.Uri}/testfile.txt"));
Assert.IsTrue(await container.GetBlobClient("testfile.txt").ExistsAsync(), "Uploaded testfile.txt blob should exist");
Assert.IsTrue(result2.Data.ContainsValue($"{container.Uri}/targetfolder/testfile.txt"));
Assert.IsTrue(await container.GetBlobClient("targetfolder/testfile.txt").ExistsAsync(), "Uploaded targetfolder/testfile.txt blob should exist");
}
else if (handler is HandleExistingFile.Overwrite)
{
var result2 = await AzureBlobStorage.UploadBlob(_source, _destinationCS, _options, default);
Assert.IsTrue(result2.Success);
Assert.IsTrue(result2.Data.ContainsValue($"{container.Uri}/testfile.txt"));
Assert.IsTrue(await container.GetBlobClient("testfile.txt").ExistsAsync(), "Uploaded testfile.txt blob should exist");
Assert.IsTrue(result2.Data.ContainsValue($"{container.Uri}/targetfolder/testfile.txt"));
Assert.IsTrue(await container.GetBlobClient("targetfolder/testfile.txt").ExistsAsync(), "Uploaded targetfolder/testfile.txt blob should exist");
}
else
{
@@ -269,10 +275,10 @@ public async Task UploadDirectory_WithAndWithoutTags()

var resultWithTags = await AzureBlobStorage.UploadBlob(_source, _destinationCS, _options, default);
Assert.IsTrue(resultWithTags.Success);
Assert.IsTrue(resultWithTags.Data.ContainsValue($"{container.Uri}/TestFiles/testfile.txt"));
Assert.IsTrue(await container.GetBlobClient("TestFiles/testfile.txt").ExistsAsync(), "Uploaded testfile.txt blob should exist");
Assert.IsTrue(resultWithTags.Data.ContainsValue($"{container.Uri}/TestFiles/testfile2.txt"));
Assert.IsTrue(await container.GetBlobClient("TestFiles/testfile2.txt").ExistsAsync(), "Uploaded testfile2.txt blob should exist");
Assert.IsTrue(resultWithTags.Data.ContainsValue($"{container.Uri}/targetfolder/TestFiles/testfile.txt"));
Assert.IsTrue(await container.GetBlobClient("targetfolder/TestFiles/testfile.txt").ExistsAsync(), "Uploaded testfile.txt blob should exist");
Assert.IsTrue(resultWithTags.Data.ContainsValue($"{container.Uri}/targetfolder/TestFiles/testfile2.txt"));
Assert.IsTrue(await container.GetBlobClient("targetfolder/TestFiles/testfile2.txt").ExistsAsync(), "Uploaded testfile2.txt blob should exist");
}
}

@@ -282,7 +288,9 @@ public async Task UploadDirectory_RenameDir()
_source.SourceType = UploadSourceType.Directory;
_source.SourceDirectory = _testFileDir;
_source.SourceFile = default;
_source.BlobFolderName = "RenameDir";
_source.RenameToFolderName = "RenameDir";
_destinationCS.TargetFolder = "";
_destinationOA.TargetFolder = "";

var container = GetBlobContainer(_connectionString, _containerName);

Original file line number Diff line number Diff line change
@@ -61,6 +61,12 @@ public class Destination
[PasswordPropertyText]
public string ClientSecret { get; set; }

/// <summary>
/// Specifies the target folder path for the blob in Azure Blob Storage.
/// This value will be prepended to blob names during upload, effectively creating a virtual directory structure.
/// </summary>
/// <example>backups/2024/</example>
public string TargetFolder { get; set; }
Comment on lines +64 to +69
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation and improve documentation for the TargetFolder property.

While the property implementation is functionally correct, consider these improvements:

  1. Add validation to ensure proper folder path format
  2. Document path format requirements (e.g., handling of slashes)
  3. Add UI hints for better user experience

Apply this diff to enhance the property:

     /// <summary>
     /// Specifies the target folder path for the blob in Azure Blob Storage.
     /// This value will be prepended to blob names during upload, effectively creating a virtual directory structure.
+    /// The path should:
+    /// - Use forward slashes (/) as directory separators
+    /// - Not start with a forward slash
+    /// - Optionally end with a forward slash (will be normalized during upload)
+    /// - Not contain invalid characters: \ " [ ] < > |
     /// </summary>
     /// <example>backups/2024/</example>
+    [DisplayFormat(DataFormatString = "Text")]
+    [RegularExpression(@"^[^\\<>\[\]""|]+(\/[^\\<>\[\]""|]+)*\/?$", 
+        ErrorMessage = "Invalid folder path format. Use forward slashes and avoid special characters.")]
+    [Description("Virtual directory path where the blob will be stored.")]
     public string TargetFolder { get; set; }
📝 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
/// <summary>
/// Specifies the target folder path for the blob in Azure Blob Storage.
/// This value will be prepended to blob names during upload, effectively creating a virtual directory structure.
/// </summary>
/// <example>backups/2024/</example>
public string TargetFolder { get; set; }
/// <summary>
/// Specifies the target folder path for the blob in Azure Blob Storage.
/// This value will be prepended to blob names during upload, effectively creating a virtual directory structure.
/// The path should:
/// - Use forward slashes (/) as directory separators
/// - Not start with a forward slash
/// - Optionally end with a forward slash (will be normalized during upload)
/// - Not contain invalid characters: \ " [ ] < > |
/// </summary>
/// <example>backups/2024/</example>
[DisplayFormat(DataFormatString = "Text")]
[RegularExpression(@"^[^\\<>\[\]""|]+(\/[^\\<>\[\]""|]+)*\/?$",
ErrorMessage = "Invalid folder path format. Use forward slashes and avoid special characters.")]
[Description("Virtual directory path where the blob will be stored.")]
public string TargetFolder { get; set; }


/// <summary>
/// Determines if the container should be created if it does not exist.
Original file line number Diff line number Diff line change
@@ -41,20 +41,20 @@ public class Source
public string SourceFile { get; set; }

/// <summary>
/// Name of the blob. If left empty, the blob's name will be the same as the source file.
/// Specifies a custom name for the blob in Azure Blob Storage. If left empty, the blob's name will be the same as the source file.
/// </summary>
/// <example>Renamed.txt</example>
[UIHint(nameof(SourceType), "", UploadSourceType.File)]
[DisplayFormat(DataFormatString = "Text")]
public string BlobName { get; set; }
public string RenameToBlobName { get; set; }

/// <summary>
/// Name of the blob folder. If left empty, blob folder's name will be the same as source directory (e.g., 'Example' in C:\temp\Example).
/// Specifies a custom name for the blob folder in Azure Blob Storage.. If left empty, blob folder's name will be the same as source directory (e.g., 'Example' in C:\temp\Example).
/// </summary>
/// <example>ExampleDir</example>
[UIHint(nameof(SourceType), "", UploadSourceType.Directory)]
[DisplayFormat(DataFormatString = "Text")]
public string BlobFolderName { get; set; }
public string RenameToFolderName { get; set; }

/// <summary>
/// Use a stream to read the file's content
Original file line number Diff line number Diff line change
@@ -33,6 +33,11 @@ public class AzureBlobStorage
/// <returns>Object { bool Success, Dictionary&lt;string, string&gt; Data }</returns>
public static async Task<Result> UploadBlob([PropertyTab] Source source, [PropertyTab] Destination destination, [PropertyTab] Options options, CancellationToken cancellationToken)
{
if (destination.ContainerName.Contains('/') || destination.ContainerName.Contains('\\'))
{
throw new Exception("The container name cannot contain '/' or '\\'.");
}
Comment on lines +36 to +39
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use a more specific exception type for container name validation

Instead of throwing a general Exception, consider throwing an ArgumentException to provide more context about the invalid ContainerName parameter.

Apply this diff to improve exception handling:

- throw new Exception("The container name cannot contain '/' or '\\'.");
+ throw new ArgumentException("The container name cannot contain '/' or '\\'.", nameof(destination.ContainerName));
📝 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
if (destination.ContainerName.Contains('/') || destination.ContainerName.Contains('\\'))
{
throw new Exception("The container name cannot contain '/' or '\\'.");
}
if (destination.ContainerName.Contains('/') || destination.ContainerName.Contains('\\'))
{
throw new ArgumentException("The container name cannot contain '/' or '\\'.", nameof(destination.ContainerName));
}


var results = new Dictionary<string, string>();

var fi = string.IsNullOrEmpty(source.SourceFile) ? null : new FileInfo(source.SourceFile);
@@ -52,8 +57,12 @@ public static async Task<Result> UploadBlob([PropertyTab] Source source, [Proper
if (fi == null)
throw new FileNotFoundException($"Source file '{source.SourceFile}' was empty.");
blobName = fi.Name;
if (!string.IsNullOrWhiteSpace(source.BlobName) || source.Compress)
blobName = RenameFile(!string.IsNullOrWhiteSpace(source.BlobName) ? source.BlobName : fi.Name, source.Compress, fi);
if (!string.IsNullOrWhiteSpace(source.RenameToBlobName) || source.Compress)
blobName = RenameFile(!string.IsNullOrWhiteSpace(source.RenameToBlobName) ? source.RenameToBlobName : fi.Name, source.Compress, fi);
if (!string.IsNullOrWhiteSpace(destination.TargetFolder))
{
blobName = $"{destination.TargetFolder.TrimEnd('/', '\\')}/{blobName}";
}
results.Add(source.SourceFile, await HandleUpload(source, destination, options, fi, blobName, cancellationToken));
break;
case UploadSourceType.Directory:
@@ -66,12 +75,17 @@ public static async Task<Result> UploadBlob([PropertyTab] Source source, [Proper
fileName = RenameFile(fileName, source.Compress, file);

var parentDirectory = Path.GetFileName(Path.GetDirectoryName(file.ToString()));
var withDir = string.IsNullOrWhiteSpace(source.BlobFolderName)
var withDir = string.IsNullOrWhiteSpace(source.RenameToFolderName)
? Path.Combine(parentDirectory, fileName)
: Path.Combine(source.BlobFolderName, fileName);
: Path.Combine(source.RenameToFolderName, fileName);

blobName = withDir.Replace("\\", "/");

if (!string.IsNullOrWhiteSpace(destination.TargetFolder))
{
blobName = $"{destination.TargetFolder.TrimEnd('/', '\\')}/{blobName}";
}

results.Add(file.FullName, await HandleUpload(source, destination, options, file, blobName, cancellationToken));
handledFile = file.FullName;
}
@@ -121,7 +135,6 @@ public static async Task<Result> UploadBlob([PropertyTab] Source source, [Proper

private static async Task<string> HandleUpload(Source source, Destination destination, Options options, FileInfo fi, string blobName, CancellationToken cancellationToken)
{
blobName = string.IsNullOrWhiteSpace(source.BlobName) ? blobName : source.BlobName;

var contentType = string.IsNullOrWhiteSpace(destination.ContentType) ? MimeUtility.GetMimeMapping(fi.Name) : destination.ContentType;
var encoding = GetEncoding(destination.FileEncoding);
@@ -383,7 +396,16 @@ private static async Task<FileInfo> AppendAny(BlobClient blob, AppendBlobClient
}
else
{
var tempFile = Path.Combine(Path.GetTempPath(), blobName);
string normalizedBlobName = blobName.Replace('/', '\\');

var tempFile = Path.Combine(Path.GetTempPath(), normalizedBlobName);

string directoryPath = Path.GetDirectoryName(tempFile);

if (!Directory.Exists(directoryPath))
{
Directory.CreateDirectory(directoryPath);
}
Comment on lines +399 to +408
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Validate blobName to prevent path traversal vulnerabilities

Using blobName directly in file paths without validation can lead to security issues if it contains path traversal characters. Sanitize blobName to ensure only valid file names are used when creating temporary files.

Apply this diff to sanitize blobName:

- string normalizedBlobName = blobName.Replace('/', '\\');
+ string normalizedBlobName = Path.GetFileName(blobName.Replace('/', '\\'));

This change uses Path.GetFileName to extract only the file name, preventing directory traversal.

📝 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
string normalizedBlobName = blobName.Replace('/', '\\');
var tempFile = Path.Combine(Path.GetTempPath(), normalizedBlobName);
string directoryPath = Path.GetDirectoryName(tempFile);
if (!Directory.Exists(directoryPath))
{
Directory.CreateDirectory(directoryPath);
}
string normalizedBlobName = Path.GetFileName(blobName.Replace('/', '\\'));
var tempFile = Path.Combine(Path.GetTempPath(), normalizedBlobName);
string directoryPath = Path.GetDirectoryName(tempFile);
if (!Directory.Exists(directoryPath))
{
Directory.CreateDirectory(directoryPath);
}


if (blob != null)
await blob.DownloadToAsync(tempFile, cancellationToken);
Loading
Oops, something went wrong.