Media.Processing MediaUrl MediaShape does not handle null/empty paths elegantly

[orchardbot]

carlwoodhouse created: https://orchard.codeplex.com/workitem/21138

If you pass an empty path to the MediaUrl shape it goes quite far through the stack till the storage provider throws an exception, then logs it as an error.

It seems quite likely that a null/empty path could be thrown at it when dealing with large datasets, propose that it would be more efficient/elegant to just return if the path is empty.

ie. [Shape] public void MediaUrl(dynamic Shape, dynamic Display, TextWriter Output, string Profile, string Path, ContentItem ContentItem, FilterRecord CustomFilter) { if (string.IsNullOrEmpty(Path)) { return; }

        try
        {
            Shape.IgnoreShapeTracer = true;
            Output.Write(_imageProfileManager.Value.GetImageProfileUrl(Path, Profile, CustomFilter, ContentItem));
        }
        catch (Exception ex)
        {
            Logger.Error(ex, "An error occured while rendering shape {0} for image {1}", Profile, Path);
        }
    }

[orchardbot]

anoordende commented:

Hi, I have created pull request 8086 as part of workitem 21235 which throws an error before processing the file.

If using one of the Media shapes (as in your code above), the error will be caught and logged. I understand that you propose to simply return, but that would hide a potential serious problem.

Please have a look at the fork to see if this fixes your issue, if not, can you please describe the exact scenario where path could be null or empty? The likelihood is that, if this is a valid case, the caller must check for the existence of a value for path, as the imageprocessor cannot possibly know the intent of the call.

[orchardbot]

carlwoodhouse commented:

I think your pr is adequate mate, i agree the caller should probably check for its existence before calling the shape.

An example as to where it could be empty ... we import a lot of thirdparty content from xml feeds, some of these might have images and some not, in which case the path for some was empty - but i think its acceptable that the view calling the shape (if using the htmlhelper) could check the path itself.