From b2a5ce3e4873a000a0038619b959c8785a400c64 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Mon, 24 Jul 2023 08:17:21 +0000 Subject: [PATCH 1/3] V10: Dropzone should handle internal and external errors when uploading (#14578) * fix: mark files that result in error as processed * fix: for safety measure check that a file is truthy before trying to upload it * fix: push an error when file.$error is encountered to make sure it does not get uploaded * fix: remove header from error messages since it is not being used anyway * fix: check for maxFileSize before uploading pasted images in tinymce * use stored blob variable * feat: add property to fileManager to get and format the maxFileSize * fix: make tinymce use fileManager to get maxFileSize * fix(image cropper): check for maxFileSize before setting file to upload * multiply by 1000 to get bytes --------- Co-authored-by: Elitsa --- .../upload/umbfiledropzone.directive.js | 33 ++++++++++++++++--- .../upload/umbpropertyfileupload.directive.js | 18 ++++++++-- .../common/services/filemanager.service.js | 11 +++++++ .../src/common/services/tinymce.service.js | 22 +++++++++---- .../components/upload/umb-file-dropzone.html | 6 ++-- 5 files changed, 72 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfiledropzone.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfiledropzone.directive.js index 98f02b7e06..25ebbd0cbc 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfiledropzone.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfiledropzone.directive.js @@ -31,7 +31,7 @@ angular.module("umbraco.directives") propertyAlias: '@', accept: '@', maxFileSize: '@', - + compact: '@', hideDropzone: '@', acceptedMediatypes: '=', @@ -87,7 +87,7 @@ angular.module("umbraco.directives") // Add the processed length, as we might be uploading in stages scope.totalQueued = scope.queue.length + scope.processed.length; - _processQueueItems(); + _processQueueItems(); } function _processQueueItems() { @@ -115,6 +115,18 @@ angular.module("umbraco.directives") function _upload(file) { + if (!file) { + return; + } + + if (file.$error) { + scope.processed.push(file); + scope.currentFile = undefined; + file.messages.push({type: "Error"}); + _processQueueItems(); + return; + } + scope.propertyAlias = scope.propertyAlias ? scope.propertyAlias : "umbracoFile"; scope.contentTypeAlias = scope.contentTypeAlias ? scope.contentTypeAlias : "Image"; @@ -158,11 +170,22 @@ angular.module("umbraco.directives") } else if (evt && typeof evt === "string") { file.messages.push({message: evt, type: "Error"}); } - // If file not found, server will return a 404 and display this message - if (status === 404) { - file.messages.push({message: "File not found", type: "Error"}); + + // If there were no errors with the request, but the status code was 404, we'll add a custom message + // or a generic message for all other status codes. + if (!file.messages.length) { + if (status === 404) { + file.messages.push({message: "File not found", type: "Error"}); + } else { + file.messages.push({message: "Error uploading file", type: "Error"}); + } } + + // The file has been processed, even though it resulted in an error, so we add it to the processed queue + scope.processed.push(file); scope.currentFile = undefined; + + // Return to queue processing _processQueueItems(); }); } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js index 7de961bb34..eebdb7c223 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js @@ -7,8 +7,10 @@ * @param {any} fileManager * @param {any} mediaHelper * @param {any} angularHelper + * @param {any} $attrs + * @param {any} notificationsService */ - function umbPropertyFileUploadController($scope, $q, fileManager, mediaHelper, angularHelper, $attrs) { + function umbPropertyFileUploadController($scope, $q, fileManager, mediaHelper, angularHelper, $attrs, notificationsService) { //NOTE: this component supports multiple files, though currently the uploader does not but perhaps sometime in the future // we'd want it to, so i'll leave the multiple file support in place @@ -271,15 +273,25 @@ if (args.files && args.files.length > 0) { + const filesAllowed = []; + + for (let i = 0; i < args.files.length; i++) { + if (fileManager.maxFileSize && args.files[i].size > fileManager.maxFileSize) { + notificationsService.error(`File upload "${args.files[i].name}"`, `File size of ${args.files[i].size / 1000} KB exceeds the maximum allowed size of ${fileManager.maxFileSize / 1000} KB`); + } else { + filesAllowed.push(args.files[i]); + } + } + //set the files collection fileManager.setFiles({ propertyAlias: vm.propertyAlias, - files: args.files, + files: filesAllowed, culture: vm.culture, segment: vm.segment }); - updateModelFromSelectedFiles(args.files).then(function(newVal) { + updateModelFromSelectedFiles(filesAllowed).then(function(newVal) { angularHelper.safeApply($scope, function() { //pass in the file names and the model files diff --git a/src/Umbraco.Web.UI.Client/src/common/services/filemanager.service.js b/src/Umbraco.Web.UI.Client/src/common/services/filemanager.service.js index 38aee3fc4a..6b2bd3c295 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/filemanager.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/filemanager.service.js @@ -14,6 +14,17 @@ function fileManager($rootScope) { var mgr = { + /** + * @ngdoc property + * @name umbraco.services.fileManager#maxFileSize + * @propertyOf umbraco.services.fileManager + * @type {Number} + * @default 0 + * @description + * The max file size allowed to be uploaded to the server in bytes + */ + maxFileSize: parseInt(Umbraco.Sys.ServerVariables.umbracoSettings.maxFileSize ?? '0', 10) * 1000, + /** * @ngdoc function * @name umbraco.services.fileManager#setFiles diff --git a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js index 584870e6e5..b9971bdee6 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js @@ -7,7 +7,7 @@ * A service containing all logic for all of the Umbraco TinyMCE plugins */ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, stylesheetResource, macroResource, macroService, - $routeParams, umbRequestHelper, angularHelper, userService, editorService, entityResource, eventsService, localStorageService, mediaHelper) { + $routeParams, umbRequestHelper, angularHelper, userService, editorService, entityResource, eventsService, localStorageService, mediaHelper, fileManager) { //These are absolutely required in order for the macros to render inline //we put these as extended elements because they get merged on top of the normal allowed elements by tiny mce @@ -222,6 +222,14 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s } function uploadImageHandler(blobInfo, success, failure, progress){ + const blob = blobInfo.blob(); + + // if the file size is greater than the max file size, reject it + if (fileManager.maxFileSize > 0 && blob.size > fileManager.maxFileSize) { + failure(`The file size (${blob.size / 1000} KB) exceeded the maximum allowed size of ${fileManager.maxFileSize / 1000} KB.`); + return; + } + const xhr = new XMLHttpRequest(); xhr.open('POST', Umbraco.Sys.ServerVariables.umbracoUrls.tinyMceApiBaseUrl + 'UploadImage'); @@ -285,7 +293,7 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s }; const formData = new FormData(); - formData.append('file', blobInfo.blob(), blobInfo.blob().name); + formData.append('file', blob, blob.name); xhr.send(formData); } @@ -727,11 +735,11 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s }; var newImage = editor.dom.createHTML('img', data); var parentElement = editor.selection.getNode().parentElement; - + if (img.caption) { var figCaption = editor.dom.createHTML('figcaption', {}, img.caption); var combined = newImage + figCaption; - + if (parentElement.nodeName !== 'FIGURE') { var fragment = editor.dom.createHTML('figure', {}, combined); editor.selection.setContent(fragment); @@ -749,7 +757,7 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s editor.selection.setContent(newImage); } } - + // Using settimeout to wait for a DoM-render, so we can find the new element by ID. $timeout(function () { @@ -770,7 +778,7 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s } }); - + } }, @@ -1454,7 +1462,7 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s // Then we need to add an event listener to the editor // That will update native browser drag & drop events // To update the icon to show you can NOT drop something into the editor - + var toolbarItems = args.editor.settings.toolbar === false ? [] : args.editor.settings.toolbar.split(" "); if(isMediaPickerEnabled(toolbarItems) === false){ // Wire up the event listener diff --git a/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-file-dropzone.html b/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-file-dropzone.html index 6581fca14d..448539f7c3 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-file-dropzone.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-file-dropzone.html @@ -41,7 +41,7 @@ From eed9feb5f305f71e4da437ea31d4b81e1b799a60 Mon Sep 17 00:00:00 2001 From: Adrian Cojocariu <95346674+acoumb@users.noreply.github.com> Date: Mon, 24 Jul 2023 16:18:23 +0300 Subject: [PATCH 2/3] Fix method invoke. (#14597) --- .../PropertyEditors/GridPropertyIndexValueFactory.cs | 4 ++-- .../PropertyEditors/RichTextPropertyEditor.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/GridPropertyIndexValueFactory.cs b/src/Umbraco.Infrastructure/PropertyEditors/GridPropertyIndexValueFactory.cs index a3bb55e643..7f6683fa59 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/GridPropertyIndexValueFactory.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/GridPropertyIndexValueFactory.cs @@ -1,4 +1,4 @@ -// Copyright (c) Umbraco. +// Copyright (c) Umbraco. // See LICENSE for more details. using System.Text; @@ -91,6 +91,6 @@ namespace Umbraco.Cms.Core.PropertyEditors [Obsolete("Use the overload that specifies availableCultures, scheduled for removal in v14")] public IEnumerable>> GetIndexValues(IProperty property, string? culture, string? segment, bool published) - => GetIndexValues(property, culture, segment, published); + => GetIndexValues(property, culture, segment, published, Enumerable.Empty()); } } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs index c11498af7a..a3a1b31c34 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs @@ -328,6 +328,6 @@ public class RichTextPropertyEditor : DataEditor [Obsolete("Use the overload with the 'availableCultures' parameter instead, scheduled for removal in v14")] public IEnumerable>> GetIndexValues(IProperty property, string? culture, string? segment, bool published) - => GetIndexValues(property, culture, segment, published); + => GetIndexValues(property, culture, segment, published, Enumerable.Empty()); } } From 43700d2bcbdcd465e1994ba90156f888459b1272 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Tue, 25 Jul 2023 08:39:59 +0200 Subject: [PATCH 3/3] Ensure that the Slider does not crash the back-office (#14601) * Ensure that the Slider does not crash the back-office * Add field descriptions to config --- .../PropertyEditors/SliderConfiguration.cs | 6 +++--- .../PropertyEditors/SliderConfigurationEditor.cs | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/PropertyEditors/SliderConfiguration.cs b/src/Umbraco.Core/PropertyEditors/SliderConfiguration.cs index 709fb3ce9f..4000af6b82 100644 --- a/src/Umbraco.Core/PropertyEditors/SliderConfiguration.cs +++ b/src/Umbraco.Core/PropertyEditors/SliderConfiguration.cs @@ -14,12 +14,12 @@ public class SliderConfiguration [ConfigurationField("initVal2", "Initial value 2", "number", Description = "Used when range is enabled")] public decimal InitialValue2 { get; set; } - [ConfigurationField("minVal", "Minimum value", "number")] + [ConfigurationField("minVal", "Minimum value", "number", Description = "Must be smaller than the Maximum value")] public decimal MinimumValue { get; set; } - [ConfigurationField("maxVal", "Maximum value", "number")] + [ConfigurationField("maxVal", "Maximum value", "number", Description = "Must be larger than the Minimum value")] public decimal MaximumValue { get; set; } - [ConfigurationField("step", "Step increments", "number")] + [ConfigurationField("step", "Step increments", "number", Description = "Must be a positive value")] public decimal StepIncrements { get; set; } } diff --git a/src/Umbraco.Core/PropertyEditors/SliderConfigurationEditor.cs b/src/Umbraco.Core/PropertyEditors/SliderConfigurationEditor.cs index 586e4cd3af..deba35012e 100644 --- a/src/Umbraco.Core/PropertyEditors/SliderConfigurationEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/SliderConfigurationEditor.cs @@ -25,4 +25,19 @@ public class SliderConfigurationEditor : ConfigurationEditor ToConfigurationEditor(SliderConfiguration? configuration) + { + // negative step increments can be configured in the back-office. they will cause the slider to + // crash the entire back-office. as we can't configure min and max values for the number prevalue + // editor, we have to this instead to limit the damage. + // logically, the step increments should be inverted instead of hardcoding them to 1, but the + // latter might point people in the direction of their misconfiguration. + if (configuration?.StepIncrements <= 0) + { + configuration.StepIncrements = 1; + } + + return base.ToConfigurationEditor(configuration); + } }