U4-8265 - Listview list and listview grid does not handle acceptedFileTypes in same way

Created by Bjarne Fyrstenborg 30 Mar 2016, 21:10:10 Updated by Bjarne Fyrstenborg 03 Apr 2016, 12:32:08

Tags: Unscheduled

Relates to: U4-8034

It seems that listview list and listview grid does not handle acceptedFileTypes in same way. Notice .replace doesn't do the same for the two.

In listview grid

   function ListViewGridLayoutController($scope, $routeParams, mediaHelper, mediaResource, $location, listViewHelper) {

      var vm = this;

      vm.nodeId = $scope.contentId;
       //vm.acceptedFileTypes = mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.imageFileTypes);
        //instead of passing in a whitelist, we pass in a blacklist by adding ! to the ext
      vm.acceptedFileTypes = mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.disallowedUploadFiles).replace(/\./g, "!.");
      vm.maxFileSize = Umbraco.Sys.ServerVariables.umbracoSettings.maxFileSize + "KB";

In listview list

    function ListViewListLayoutController($scope, listViewHelper, $location, mediaHelper) {

        var vm = this;

        vm.nodeId = $scope.contentId;
        //vm.acceptedFileTypes = mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.imageFileTypes);
        //instead of passing in a whitelist, we pass in a blacklist by adding ! to the ext
        vm.acceptedFileTypes = mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.disallowedUploadFiles).replace(/./g, "!.");
        vm.maxFileSize = Umbraco.Sys.ServerVariables.umbracoSettings.maxFileSize + "KB";

Output to console log console.log("ListViewGridLayout acceptedFileTypes", vm.acceptedFileTypes);

ListViewGridLayout acceptedFileTypes !.ashx,!.aspx,!.ascx,!.config,!.cshtml,!.vbhtml,!.asmx,!.air,!.axd,!.swf,!.xml,!.html,!.htm,!.svg,!.php,!.htaccess

console.log("ListViewListLayout acceptedFileTypes", vm.acceptedFileTypes);

ListViewListLayout acceptedFileTypes !.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.

I also have has an issue with my package Color Palettes in 7.4.x .. https://our.umbraco.org/projects/backoffice-extensions/color-palettes/bugs-feedback-and-suggestions/76218-breaks-media-drag-and-drop

it works in 7.2.x and 7.3.x.. but when Color Palettes is installed I get these error messages even though the upload directive has limited files to .json and .txt https://github.com/bjarnef/color-palettes/blob/master/ColorPalettes/ColorPalettes/ColorPalettes/views/colorpalettes.settings.html#L13-L15

It use ng-file-upload Angular module, which I think it the one Umbraco also use now: https://github.com/bjarnef/color-palettes/tree/master/ColorPalettes/ColorPalettes/ColorPalettes/modules

I wonder which changes has been made from 7.3.x -> 7.4.x that might cause this error?

Anyway, also notice the screenshots and the different error messages depending on upload in grid or list mode.

Finally it is not very informative to add "!" in front of each file extension - at least not in the feedback message to the editors.

5 Attachments

Comments

Bjarne Fyrstenborg 30 Mar 2016, 21:28:53

I think this commit might have affected the issue https://github.com/umbraco/Umbraco-CMS/commit/e807ee7d643c8ce73158144244feb81f57c2a4df

but it doesn't match the same format in grid layout controller: https://github.com/umbraco/Umbraco-CMS/search?l=javascript&q=disallowedUploadFiles&utf8=%E2%9C%93

and it doesn't handle it the same way in mediapicker controller: https://github.com/umbraco/Umbraco-CMS/blob/8bfb77d3fec843185cb6617343c23db146a27dd4/src/Umbraco.Web.UI.Client/src/views/common/overlays/mediaPicker/mediapicker.controller.js#L14


Bjarne Fyrstenborg 30 Mar 2016, 21:32:33

I am not sure which version of ng-file-upload Umbraco use, but I think the "accept" attribute expect the allowed file extenstion https://github.com/danialfarid/ng-file-upload/issues/658


Sebastiaan Janssen 02 Apr 2016, 12:19:16

The ! means NOT allowed, this is a blacklist, not a whitelist. So yeah, that whole variable is wrongly named and this whole thing is super awkward.

From reading this I cannot figure out what's actually wrong for you? I've tried uploading a pdf and images in both listview layouts and in a media picker, all of them reject the pdf when I add it to the disallowedUploadFiles list, all of them accept the pdf when it's not in the disallowedUploadFiles list. Let me know what's actually wrong for you!

Obviously the ListViewListLayout needs the updated regex too, but it still seems to work somehow.


Bjarne Fyrstenborg 02 Apr 2016, 12:45:11

@sebastiaan yes, but because of the blacklist the error feedback message says "(Only allowed file types: !.ashx, !.aspx, ...") see screenshots.

But somehow it also seems that the changes for this in 7.4.x affect my package Color Palettes where I accept .json and .txt. Or when Color Palettes is installed I can't upload e.g. .jpg files to media section.

I am not sure if it is because I also use ng-file-upload directive, because I accept "json and txt" and which might affect the "dropzone" upload in core or what cause the issue.


Sebastiaan Janssen 02 Apr 2016, 12:58:53

Are txt or json in your disallowedUploadFiles list? If not, can you fix the ListViewListLayout with the correct regex: vm.acceptedFileTypes = mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.disallowedUploadFiles).replace(/\./g, "!."); so it returns the same list as the grid version? Make sure to update the ClientDependency version after you do that. Then try again in Color Pallettes?

I have a feeling it's got nothing to do with that regex though.


Sebastiaan Janssen 02 Apr 2016, 13:01:12

Maybe you use Umbraco.Overlays.MediaPickerController though? See U4-8034 and try to fix that in the same way as suggested in the PR?


Asbjørn Riis-Knudsen 02 Apr 2016, 17:01:21

Isn't there a backslash missing in the list layout controller: vm.acceptedFileTypes = mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.disallowedUploadFiles).replace(/./g, "!.");

Compared to grid view: vm.acceptedFileTypes = mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.disallowedUploadFiles).replace(/./g, "!.");

The dot should be escaped, since the regex is search for a literal dot in the file extension.


Sebastiaan Janssen 02 Apr 2016, 17:12:38

Indeed :)


Bjarne Fyrstenborg 02 Apr 2016, 20:10:48

Hi @sebastiaan No .json and .txt are only added to ngf-pattern attribute for the directive https://github.com/bjarnef/color-palettes/blob/master/ColorPalettes/ColorPalettes/ColorPalettes/views/colorpalettes.settings.html#L13

Okay, will try to check if changing the regex makes a difference.

It was an issue both when uploading files/images via media picker and in media section.


Bjarne Fyrstenborg 02 Apr 2016, 20:36:28

Oh, sorry .. I can upload images via media picker in v. 7.4.2 when Color Palettes is installed, but not in media section no matter it is listview grid or list layout.


Bjarne Fyrstenborg 02 Apr 2016, 20:46:56

Making the change in ListViewListLayout doesn't fix the issue for uploading files with Color Palettes installed, but returns the same error feedback messages as in listview grid layout, although "(The file types are not allowed: ".ashx, .aspx, .ascx, .config, .cshtml, .vbhtml, .asmx, .air, .axd, .swf, .xml, .html, .htm, .svg, .php, .htaccess")" since it is changed to a blacklist.

I can't upload files like .jpg or .json and .txt (allowed in Color Palettes), but shouldn't have affect on fileupload in "dropzone".


Bjarne Fyrstenborg 02 Apr 2016, 21:02:20

@sebastiaan I am not sure if this is right? but if I in listview change to this:

vm.acceptedFileTypes = !mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.disallowedUploadFiles);

then I can upload .jpg, .png, .json, .txt etc. to media section when Color Palettes is installed, but not disallowed files like .aspx, .html, .xml, .swf etc.


Sebastiaan Janssen 03 Apr 2016, 11:12:52

I think your only problem is that you're bundling modules/ng-file-upload.js. When I remove that from your package manifest file, the uploads work perfectly again and I can't see any problems with your package import/export functionality when that module is not in your package manifest.

As for the difference between grid/list, this is now fixed in https://github.com/umbraco/Umbraco-CMS/commit/65024707086fb5554fd1a837ffa23bc1fc4e5a6c The error message is now also not silly any more and the same for js and server side validation (see attached).


Bjarne Fyrstenborg 03 Apr 2016, 11:37:58

Thanks for taking your time to look at this issue @sebastiaan Do you know when ng-file-upload was added in core? Maybe it was another module used previously as it wasn't an issue in 7.2.x and 7.2.x

So I don't need to register the module, when it is already used in core, but I think it wasn't included previously?


Sebastiaan Janssen 03 Apr 2016, 11:39:14

By the way, you're totally right, for some insane reasion doing just vm.acceptedFileTypes = !mediaHelper.formatFileTypes..etc totally works. I've updated that now: https://github.com/umbraco/Umbraco-CMS/commit/d8c781f97f344792b2d21936605f2045baa112cf


Sebastiaan Janssen 03 Apr 2016, 11:42:33

No worries! It was included in 7.4 I imagine, for the media section refresh. Anyway, I've re-included your module dependency as well now and it seems to all work with the current color palettes package too. If you can double check when this finished building in 15 minutes that would be awesome: https://ci.appveyor.com/project/Umbraco/umbraco-cms-hs8dx/build/3488/artifacts


Bjarne Fyrstenborg 03 Apr 2016, 11:49:25

Awesome! I'll double check in that build. For 7.4.x it is not necessary to include ng-file-upload, but would be great if it doesn't break if package developers include it anyway. For people already using it in 7.4.0 - 7.4.2 they should probably just exclude ng-file-upload from the package.manifest


Bjarne Fyrstenborg 03 Apr 2016, 12:32:08

I just tested it with the new build and it works great for both media picker upload, upload via listview list and grid layout... and both with and without Color Palettes installed.

Finally it also respect Color Palettes only accept .txt and .json file upload.


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.4.2

Due in version: 7.4.3

Sprint:

Story Points:

Cycle: