Unescaped hash "#" in media filename

[dblood2]

Using AzureBlobStorage in a Piranha 8.3 project. Azure allows the "#" symbol in the filename. Piranha uploads the images just fine, but fails when attempting to display the image in the site and manager.

Real-world example: my user added 1,000 images to a media folder, all with a hash included like so: filename_#4_002.jpg.
The filename in Azure shows the hash, the Public Uri in Azure shows the hash, but when Piranha attempts to display the image, it gives a 404 error.

Current Workaround: Encoding the "#" to "%23" in both the Gallery and MediaPicker Partials allows the image to work. Unfortunately, when using an ImageBlock, I can only change the DisplayTemplate which allows the image to work within the site.
For the manager, I'm forced to either edit the core code, or create my own ImageBlock. I have created my own ImageBlock, but can't figure out how to turn off the inherent ImageBlock. I don't want my user to pick the wrong one.

Not sure if this would be considered a bug or not, just respectfully pointing out an issue I've encountered.

[tidyui]

Hi there! As the storage providers are responsible for generating the public url, the simplest and most correct way to fix this would be to update BlobStorage.cs here (https://github.com/PiranhaCMS/piranha.core/blob/master/core/Piranha.Azure.BlobStorage/BlobStorage.cs#L128) to this:

public string GetResourceName(Media media, string filename)
{
    if (_naming == BlobStorageNaming.UniqueFileNames)
    {
        return $"{ media.Id }-{ System.Web.HttpUtility.UrlEncode(filename) }";
    }
    else
    {
        return $"{ media.Id }/{ System.Web.HttpUtility.UrlEncode(filename) }";
    }
}

As for removing a default block the following would do the trick:

App.Blocks.UnRegister<Piranha.Extend.Blocks.ImageBlock>();

[tidyui]

Until it’s fixed you could easily make your own version of the BlobStorage package with the UrlEncode fix!

[dblood2]

@tidyui- Thank you for the info! I will create the package and report back to confirm the fix.

[tidyui]

Excellent, I have confirmed the fix locally in the manager by performing the same fix in the FileStorage package!

[tidyui]

Sorry... The solution doesn't seem to be 100% accurate, it only works for already uploaded files as GetResourceName() is also used when generating the filenames on disk. I'll come back with a solution only affection the GetPublicUrl() method

[tidyui]

So the best solution that you can do right now is this. Replace the method GetResourceName() with the following two methods.

/// <summary>
/// Gets the resource name for the given media object.
/// </summary>
/// <param name="media">The media file</param>
/// <param name="filename">The file name</param>
/// <returns>The public url</returns>
public string GetResourceName(Media media, string filename)
{
    return GetResourceName(media, filename, false);
}

/// <summary>
/// Gets the resource name for the given media object.
/// </summary>
/// <param name="media">The media file</param>
/// <param name="filename">The file name</param>
/// <param name="encode">If the filename should be URL encoded</param>
/// <returns>The public url</returns>
public string GetResourceName(Media media, string filename, bool encode)
{
    if (_naming == BlobStorageNaming.UniqueFileNames)
    {
        return $"{ media.Id }-{ (encode ? System.Web.HttpUtility.UrlEncode(filename) : filename) }";
    }
    else
    {
        return $"{ media.Id }/{ (encode ? System.Web.HttpUtility.UrlEncode(filename) : filename) }";
    }
}

This will make sure that external calls to GetResourceName() doesn't get an escaped value back. Then change the call to the method from GetPublicUrl() to:

return $"{ _containerUrl }/{ GetResourceName(media, id, true) }";

This should fix issues with currently uploaded files and allow new files with special characters to be uploaded. The only thing that won't work is that right after upload, the thumbnail for uploaded files with special characters will be broken in the manager. A reload of the media list page fixes the issue. This is due to the fact that one logical case in the MediaService happens to return GetResourceName() instead of GetPublicUrl(). We will fix this in the official fix for the problem.

Best regards

[dblood2]

Wow - that was fast. Thank you again - should've put this in a few days ago, but instead beat my head against the desk trying to make sure it was an actual bug. Amazing response as always!

[dblood2]

@tidyui - When I change the signature of the GetResourceName method, I get a build error in the IStorage interface since BlobStorage is inheriting from it. If I change the signature of IStorage, I get a build error from MediaService.cs. If I change MediaService, Piranha.Azure.BlobStorage builds.

But, when attempting to publish the library, Piranha.Azure.BlobStorageSession fails with 4 calls to GetResourceName that also must be changed. After editing those, I was able to publish the dll. Unfortunately, I had cloned 9.0.0 instead of the version I'm on: 8.4 (which I incorrectly reported as 8.3 above). I'm cloning 8.4 now and will make the edits there, republish, and report back.

If I'm not following your instructions correctly, please push me back on the proper path.

[dblood2]

My bad, I see that you overloaded the method for GetResourceName. I did it wrong.

I can confirm that the fix, if properly executed, works. I did, however, have to install both Microsoft.Azure.Storage.Blob v11.1.0 and Microsoft.Azure.Storage.Common v11.1.0 NuGet packages to satisfy dependencies of the new library I built with the changes incorporated.

@tidyui - Thanks again - I promise I won't open this back up when you close it this time.