Workaround for file extension out from filename.slice() with "." inside

[gdessimoneinvalle]

https://github.com/OrchardCMS/Orchard/blob/97648ed5a2a6c43074216786126ff8a207e6ac2f/src/Orchard.Web/Modules/Orchard.Media/Views/Admin/Add.cshtml#L45

In some cases the file name out from filename.slice(filename.lastIndexOf(".")).toLowerCase() still contains the char "." and fails the next check, so replacing it with an empty string fix the problem

[sebastienros]

I think the code is doing what it intends to do, by keeping the . from the extension. The issue is that the documentation should mention that the setting should contain dots. I think it matters because otherwise some file extensions might be forged because we are checking that the configured ones contain the same string. Note that the current code would allow .pn extensions if you allowed .png and I think this is an issue. We should check that the exact extension is in the list, of a substring of it.

[MatteoPiovanelli-Laser]

I would also add something else: The proposed change is to a file in Orchard.Media rather than Orchard.MediaLibrary. The example files extensions in the hint text from the Orchard.MediaLibrary feature all start with ., even though there's no explicit remark to the fact a dot is required. The test is then done with this method https://github.com/OrchardCMS/Orchard/blob/97648ed5a2a6c43074216786126ff8a207e6ac2f/src/Orchard.Web/Modules/Orchard.MediaLibrary/Models/MediaLibrarySettingsPart.cs#L21 The . is kept (only extensions starting with it are considered). Then the test is filename.EndsWith(e, StringComparison.OrdinalIgnoreCase), which should account for the remark about possible substrings of the extension.

[BenedekFarkas]

@gdessimoneinvalle are you still using Orchard.Media actively / in production? Can't you upgrade to Orchard.MediaLibrary?

[BenedekFarkas]

@gdessimoneinvalle see my question above and:

In some cases the file name out from filename.slice(filename.lastIndexOf(".")).toLowerCase() still contains the char "." and fails the next check, so replacing it with an empty string fix the problem

Can you provide examples to repro?

[sebastienros]

@gdessimoneinvalle see my question above and:

In some cases the file name out from filename.slice(filename.lastIndexOf(".")).toLowerCase() still contains the char "." and fails the next check, so replacing it with an empty string fix the problem

Can you provide examples to repro?

Possible if the filename contains more than one "."

[BenedekFarkas]

I figured that much, but looking at the code I don't see how the proposed change would affect anything.

The original line is: var ext = filename.slice(filename.lastIndexOf(".")).toLowerCase(); So ext will be whatever comes after the last dot.

After the change: var ext = filename.slice(filename.lastIndexOf(".")).toLowerCase().replace('.',''); it would remove the remaining dots, but there can't be any.

I tried uploading files using the parent commit of the PR (97648ed5a2a6c43074216786126ff8a207e6ac2f) and all of my test cases (lorem.ipsum.txt, lorem...ipsum...txt, .gitignore) worked fine.

No repro, @gdessimoneinvalle please reopen if you can provide one.