Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Certain (bad) Data List configurations result in an obscure Null Reference Exception. #311

Closed
1 task
JasonElkin opened this issue Mar 6, 2023 · 3 comments
Assignees
Labels
bug-report Something isn't working
Milestone

Comments

@JasonElkin
Copy link

JasonElkin commented Mar 6, 2023

Which Contentment version are you using?

4.4.2

Which Umbraco version are you using? For example: 8.14.1 - don't just write v8

10.4.1

Bug summary

Certain configurations of the Data List editor (and possibly others, I haven't checked) can result in the Property Value Converter returning a null for GetPropertyValueType.

This is the same as the issue as reported here: umbraco/Umbraco-CMS#13515

Umbraco should probably null check the type from the Property Value Converters and handle more gracefully, but I think it would be friendlier if contentment could make it harder to create a broken property.

Steps to reproduce

Simplest way is:

  1. Create a new Data List property.
  2. Use the Enum data source, but don't choose an Enum.
  3. Save and publish.

Expected result / actual result

I wouldn't expect the editor to work, but I think I'd expect a more graceful failure (i.e. not breaking the whole document) but, in an ideal world, there would be some validation to stop an editor from doing this (or at least make it harder).

Update: Umbraco won't throw for this and break the document now. Though, strictly speaking, the Property Value Converter still shouldn't return null for the type.

Do you have Umbraco ModelsBuilder enabled?

  • Yes, it is enabled.

What browsers are you seeing the problem on?

Firefox, Chrome, Safari, Microsoft Edge

@JasonElkin JasonElkin added the bug-report Something isn't working label Mar 6, 2023
@JasonElkin
Copy link
Author

There will be a type check in place in Umbraco from 11.3.0 that will stop this from breaking the document: umbraco/Umbraco-CMS#13553

@leekelleher
Copy link
Owner

leekelleher commented Mar 8, 2023

@JasonElkin Thank you for the update. 😁

I'll put something in place on Contentment's side too - once Umbraco Spark week is out the way! 😬

@leekelleher leekelleher added this to the 4.5.0 milestone Apr 15, 2023
@leekelleher
Copy link
Owner

I took a little longer than post-Spark, had a holiday too. I've replaced the potential null with a default fallback (for Data List that'll be a type(string)). This will be in the next patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-report Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants