"Unexpected character encountered while parsing value" on save and publish

[kows]

"Unexpected character encountered while parsing value" on save and publish

[github-actions]

Hi there @kows!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[Migaroez]

Hey @kows do I understand correctly that

If you manipulate the content model in SendingContentNotification by either

• marking a blocklist based property as readonly
• or make the property not belong to any propertycollections

And then save OR Save & publish, you get the error you described?

If so, could you give me a (trimmed down) code example of your notificationHandler?

[kows]

Yes and the block list editor must contain data for it to happen.

internal sealed class SendingContentNotificationHandler()
	: INotificationHandler<SendingContentNotification>
{
	public void Handle(SendingContentNotification notification)
	{
		if (notification.Content.ContentTypeAlias.Equals(Website.ModelTypeAlias))
		{
			foreach (ContentVariantDisplay variant in notification.Content.Variants)
			{
                                //exclude 'groupToHideAlias' tab/group which contains a blocklist editor
				variant.Tabs = variant.Tabs.Where(x => !x.Alias.InvariantEquals("groupToHideAlias"));
			}
		}

		if (notification.Content.ContentTypeAlias.Equals(ContentPage.ModelTypeAlias))
		{
			foreach (ContentVariantDisplay variant in notification.Content.Variants)
			{
				//making all editors on tabs readonly except 'Generic' tab
				foreach (Tab<ContentPropertyDisplay> tab in variant.Tabs.Where(t => t.Type == "Tab"))
				{
					foreach (ContentPropertyDisplay prop in tab.Properties ?? [])
					{
						prop.Readonly = true;
					}
				}
			}
		}
	}
}

[Migaroez]

Hey @kows I had a look and was able to reproduce this with the following simplified NotificationHandler

using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Notifications;

namespace Umbraco.Cms.Web.UI.App_Data;

public class issue16288 : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.AddNotificationHandler<SendingContentNotification, ReadOnlyBlockSendingNotificationHandler>();
    }
}

public class ReadOnlyBlockSendingNotificationHandler : INotificationHandler<SendingContentNotification>
{
    public void Handle(SendingContentNotification notification)
    {
        ContentPropertyDisplay? property =
            notification.Content.Variants
                .SelectMany(v => v.Tabs)
                .SelectMany(t => t.Properties ?? [])
                .FirstOrDefault(p => p.Alias == "blocklist");
        if (property is not null && property.SupportsReadOnly)
        {
            property.Readonly = true;
        }
    }
}

Looking at the code that processes property updates, we have some code in place that checks whether a property editor is marked as read-only. When you however mark the property as read-only/remove it from the outbound model, the client does not send any property data back for those properties while the property editor expects there to be a value.

In short: We do not expect you to change that value. The reason why this works for other editors and not this one is complex.

Because the system was not purposefully designed to handle this kind of manipulation, I am going to close this bug report. Feel free to open a discussion to express the use case where you would need this kind of functionality.

[kows]

@Migaroez
whenever you want to tie security towards specific properties?
There's even a package out there: https://marketplace.umbraco.com/package/our.umbraco.community.adminonlyproperty
Kind of weird to say Because the system was not purposefully designed to handle this kind of manipulation, I am going to close this bug report. if you have a Readonly flag.

EDIT: this was no issue in older versions with Nested Content.

[Migaroez]

I do not deem setting an outbound (display) model flag as a way of enforcing security

To make something truly secure (in an editing context), you need to control the inbound flow, and the outbound just to be friendly. If you only control the outbound, you end up with security by obscurity as shown in the video below. To save time I used the package you linked with 2 users. But it should (and I am pretty sure it will) work the same with marking it read-only as I tested the client's behaviour regarding payload construction previously.

16288_obscurity.mp4

With purposefully designed I meant: I do not think all possible outcomes were considered when a feature was build that gave you the outbound model and just said "hey, update this however you want".

I can only guess as I was not a core developer at the time, but I believe the intent was that if you were to change anything or all things on an outbound model, you would make sure it wouldn't brake things when the client then send it back. And I am pretty sure that there was no clear intent to allow this feature to support implementers make things "more secure" (as per previous statement and example)