U4-9957 - String containing [] is not a valid udi.

Created by Mikkel Holck Madsen 24 May 2017, 08:17:30 Updated by Mikkel Holck Madsen 26 May 2017, 11:14:51

Tags: Unscheduled

Subtask of: deploy-318

When using Umbraco Deploy, we have an issue for certain types, where we allow [] in the file names, i.e. in a partial view macro.

so it is possible to create files using [] in their file names. The filenames makes up the udi, and then it doesnt parse as a valid udi.

I.e. for LinqPad, the code is taken from the Udi class and the StringUdi class.:

void Main()
{
    var name = "/media/37519/sdc12882[1].jpg";
    
    var escaped = Uri.EscapeUriString(name);
    escaped.Dump();
    var udi = "umb://media-file/" + escaped;
    Uri.IsWellFormedUriString(udi, UriKind.Absolute).Dump();
}

It seems like using Uri.EscapeUriString, doesnt handle the [] chars. Ive tried with Uri.EscapeDataString, and that works, but creates some really ugly udis, i.e. the /'s are converted to special chars. The good thing is that it passes the Uri.IsWellFormedUriString.

To reproduce, have a site containing Deploy in it. Create a PartialViewMacro file called View[1].cshtml. Then create a macro, that is using the file. That will generate a uda file for the Macro, that contains a dependency for the PartialViewMacro, with a udi of: umb://partial-view-macro/View[1].cshtml.

Once that gets parsed, either by running echo > deploy locally, or pushing it to the cloud(where it does the same), it will fail, as it tries to verify the udi.

I've seen it as well for sites that are being upgraded, where they might have some media files that are named with a [] signs in their names. It is not allowed on new sites, but for sites created when it was allowed, these chars can be in the names.

Comments

Stephan 24 May 2017, 09:21:29

This is actually a Cms fix since it deals with how UDI are encoded.

PR https://github.com/umbraco/Umbraco-CMS/pull/1963


Mikkel Holck Madsen 26 May 2017, 11:14:02

Moved this to the Core tracker

Pr looks fine, AppVeyer is kinda confused, but seem to be something it gets wrong

Mergin it it


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty:

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.6.0, 7.6.1

Due in version: 7.6.2

Sprint: Sprint 59

Story Points:

Cycle: 1