Skip to content

BUG: Normalisation of file ids in Azure implementation is a footgun on url methods #61

Description

@rabuckley

With this code

// private readonly ICloudFileStorage _blobStorage;

var stored = await _blobStorage.UploadFileAsync(fileName, stream, "image/jpeg", cancellationToken: cancellationToken);
var url = await _blobStorage.GetDownloadUrlAsync(stored.Id.ToString());

you don't find the file you've just uploaded because the AzureFileStorage implementation has normalised it to uppercase

protected async Task<BlobClient> GetBlobClientAsync(FileRecord.ID fileId)
{
var cloudBlobContainer = await _defaultBlobContainerClient.Task.ConfigureAwait(false);
return cloudBlobContainer.GetBlobClient(GetBlobName(fileId));
}
protected static string GetBlobName(FileRecord.ID fileId)
=> fileId.ToString().ToUpperInvariant();

Potential fixes

Unfortunately, removing the normalisation would be a huge breaking change.

There's a couple of potential ways to fix,

  1. Apply .ToUpperInvariant() in GetSasUriAsync method before passing to helper methods. This is unlikely to break consumers, unless they're using the method to retrieve URLs for lowercased files that where uploaded without using the AzureFileStorage implementation)
  2. Refactor method signatures to use FileRecord.ID type instead of strings, with these consistently normalised by the implementation. (Source breaking change)

For option 2, this would look like

public interface ICloudFileStorage : IFileStorage
{
    /// <summary>
    /// Request the Download URL from a cloud storage blob, given a file ID.
    /// </summary>    
    Task<Uri> GetDownloadUrlAsync(FileRecord.ID fileId, CancellationToken cancellationToken = default);

    /// <summary>
    /// Request the Upload URL from a cloud storage blob, given a file ID.
    /// </summary>    
    Task<Uri> GetUploadUrlAsync(FileRecord.ID fileId, CancellationToken cancellationToken = default);

    /// <summary>
    /// Request the Delete URL from a cloud storage blob, given a file ID.
    /// </summary>
    Task<Uri> GetDeleteUrlAsync(FileRecord.ID fileId, CancellationToken cancellationToken = default);

   // ... rest of the interface unchanged
}

GetSasUriAsync would take FileRecord.Id too, and would call

return ContainerHelper.GetSharedAccessUriFromContainer(
+   GetBlobName(blobFileId),
    action,
    containerClient,
    _cloudFileStorageOptions.SharedAccessDuration!.Value);

doing the normalisation there.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions