umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.37k stars 2.64k forks source link

Sorting fails for content with MultiUrlPicker, upon deserializing link with unexpected character #8508

Closed andreasrangholm closed 2 years ago

andreasrangholm commented 3 years ago

Attempt at sorting content failed with error message: 'Failed to sort content'

Umbraco version

I am seeing this issue on Umbraco version: 8.6.3

Reproduction

Right-clicked parent node. Left-clicked 'Sort'. Left-click-hold and dragged top node in list to second-from-the-top placement. Left-clicked 'Save' button. Error message 'Failed to sort content' appeared.

Bug summary

Based on the ExceptionMessage (see below) I believe on one of the nodes to be sorted, there might have been filled out a 'Link' value in an element of the MultiUrlPicker, that has a character that can't be parsed upon calling Newtonsoft.Json.JsonConvert.DeserializeObject<List<LinkDto>>(asString). It might be possible to either validate or sanitize the value upon submitting a newly added link for a MultiUrlPicker it in the Umbraco backoffice.

**Request URL**: <client-url>/umbraco/backoffice/UmbracoApi/Content/PostSort
**Status Code**: 500
**Response**:
{
"Message": "An error has occurred.",
"ExceptionMessage": "Unexpected character encountered while parsing value: h. Path '', line 0, position 0.",
"ExceptionType": "Newtonsoft.Json.JsonReaderException",
"StackTrace": "   at Newtonsoft.Json.JsonTextReader.ParseValue()
   at Newtonsoft.Json.JsonReader.ReadForType(JsonContract contract, Boolean hasConverter)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
   at **Umbraco.Web.PropertyEditors.MultiUrlPickerValueEditor.<GetReferences>d__7.MoveNext() in d:\\a\\1\\s\\src\\Umbraco.Web\\PropertyEditors\\MultiUrlPickerValueEditor.cs:line 185**
   at Umbraco.Core.PropertyEditors.DataValueReferenceFactoryCollection.GetAllReferences(PropertyCollection properties, PropertyEditorCollection propertyEditors) in d:\\a\\1\\s\\src\\Umbraco.Core\\PropertyEditors\\DataValueReferenceFactoryCollection.cs:line 33
   at Umbraco.Core.Persistence.Repositories.Implement.ContentRepositoryBase`3.PersistRelations(TEntity entity) in d:\\a\\1\\s\\src\\Umbraco.Core\\Persistence\\Repositories\\Implement\\ContentRepositoryBase.cs:line 829
   at Umbraco.Core.Persistence.Repositories.Implement.DocumentRepository.PersistUpdatedItem(IContent entity) in d:\\a\\1\\s\\src\\Umbraco.Core\\Persistence\\Repositories\\Implement\\DocumentRepository.cs:line 712
   at Umbraco.Core.Cache.DefaultRepositoryCachePolicy`2.Update(TEntity entity, Action`1 persistUpdated) in d:\\a\\1\\s\\src\\Umbraco.Core\\Cache\\DefaultRepositoryCachePolicy.cs:line 128
   at Umbraco.Core.Services.Implement.ContentService.Sort(IScope scope, IContent[] itemsA, Int32 userId, EventMessages evtMsgs, Boolean raiseEvents) in d:\\a\\1\\s\\src\\Umbraco.Core\\Services\\Implement\\ContentService.cs:line 2358
   at Umbraco.Core.Services.Implement.ContentService.Sort(IEnumerable`1 ids, Int32 userId, Boolean raiseEvents) in d:\\a\\1\\s\\src\\Umbraco.Core\\Services\\Implement\\ContentService.cs:line 2313
   at Umbraco.Web.Editors.ContentController.PostSort(ContentSortOrder sorted) in d:\\a\\1\\s\\src\\Umbraco.Web\\Editors\\ContentController.cs:line 1532
   at lambda_method(Closure , Object , Object[] )
   at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ActionExecutor.<>c__DisplayClass6_2.<GetExecutor>b__2(Object instance, Object[] methodParameters)
   at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ExecuteAsync(HttpControllerContext controllerContext, IDictionary`2 arguments, CancellationToken cancellationToken)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.ApiControllerActionInvoker.<InvokeActionAsyncCore>d__1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.ActionFilterResult.<ExecuteAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.ExceptionFilterResult.<ExecuteAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Web.Http.Controllers.ExceptionFilterResult.<ExecuteAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Dispatcher.HttpControllerDispatcher.<SendAsync>d__15.MoveNext()"
}

Specifics

Umbraco 8.6.3 Browser: Chrome (Version 84.0.4147.89 (Official Build) (64-bit)) Checked on other browser: Firefox Developer Edition (Version 80.0b1 (64-bit))

Steps to reproduce

Right-clicked parent node. Left-clicked 'Sort'. Left-click-hold and dragged top node in list to second-from-the-top placement. Left-clicked 'Save' button. Error message 'Failed to sort content' appeared.

Expected result

Sorting would succeed and upon reloading, the nodes would appear in sorted order.

Actual result

Sorting fails, an error message appears and upon reloading, nodes has not been sorted.

nul800sebastiaan commented 3 years ago

@andreasrangholm That's an unfortunate problem! Luckily, for most people, this is not a problem so I believe there's something specific in your stored data that's causing this. In order to look at this we do need to know what the data is that is giving you problems, could you investigate that some more please?

andreasrangholm commented 3 years ago

@nul800sebastiaan Sorry for the delay. I've investigated some more. Based on the following database query I copied the values from the 'data' column to a JSON file.

SELECT TOP (1000) ccn.[nodeId]
      ,[published]
      ,[data]
      ,[rv]
      ,un.[text]
      ,un.[trashed]
FROM [clientdatabasename].[dbo].[cmsContentNu] as ccn
INNER JOIN [clientdatabasename].[dbo].[umbracoNode] un on un.[Id] = ccn.nodeId
WHERE [data] like '%externalLinks%' and published = 1 and un.[trashed] = 0
ORDER BY [nodeId] asc`

I then tried deserializing the data, similarly to how it is done in Umbraco.Web.PropertyEditors.MultiUrlPickerValueEditor.GetReferences(). I encountered errors for data with the following format

{
        "properties": {
            "company": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "Person name"
                }
            ],
            "fullName": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "Person name"
                }
            ],
            "workTitle": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "Gastronomisk iværksætter"
                }
            ],
            "picture": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "umb://media/removed-unique-id"
                }
            ],
            "description": [],
            "externalLinks": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "https://www.linkedin.com/in/removed-profile-name-11111/"
                }
            ]
        },
        "cultureData": {},
        "urlSegment": "person-name"
    }

and

{
        "properties": {
            "company": [],
            "fullName": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "Employee name"
                }
            ],
            "workTitle": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "IKT-leder & BIM-specialist"
                }
            ],
            "picture": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "umb://media/removed-unique-id"
                }
            ],
            "description": [],
            "externalLinks": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "https://www.removedcompanyname.dk/removed-work-title/removed-employee-name/"
                }
            ]
        },
        "cultureData": {},
        "urlSegment": "employee-name"
    }

I compared them to other externalLinks properties that I was able to deserialize and they all had a format similar to the following

[...]
"externalLinks": [
                {
                    "culture": "",
                    "seg": "",
                    "val": "[{\"name\":\"Se profil\",\"target\":\"_blank\",\"url\":\"https://www.linkedin.com/in/linkedin-id/\"}]"
                }
            ]
[...]

which seems to match the object expected to be deserialized in the line from Umbraco.Web.PropertyEditors.MultiUrlPickerValueEditor.GetReferences() where the following call is made JsonConvert.DeserializeObject<List<LinkDto>>(asString)

So it seems to me that the problem might be that multi-url picker values are sometimes saved as URLs and other times as lists of links.

umbrabot commented 2 years ago

Hiya @andreasrangholm,

Just wanted to let you know that we noticed that this issue got a bit stale and might not be relevant any more.

We will close this issue for now but we're happy to open it up again if you think it's still relevant (for example: it's a feature request that's not yet implemented, or it's a bug that's not yet been fixed).

To open it this issue up again, you can write @umbrabot still relevant in a new comment as the first line. It would be super helpful for us if on the next line you could let us know why you think it's still relevant.

For example:

@umbrabot still relevant This bug can still be reproduced in version x.y.z

This will reopen the issue in the next few hours.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face: