16640 fix JSON custom field save nul
Fixes: #16640
This will fix new saves, doesn't go back and fix old values - could set it to UI editable and edit it to fix. If you have saved multiple times then it will keep escaping so not sure if more dangerous to auto-fix with a validate as JSON and if not valid then revert to ''...
Hi all, I am the reported of the original issue. This PR appears to fix the issue for null values only. Editing existing values, or creating a prefix with a default value both lead to escaped strings being placed in the field
This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken.
@FliesLikeABrick could you please give instructions for reproducing a case which the PR does not address?
@FliesLikeABrick could you please give instructions for reproducing a case which the PR does not address?
Any non-null custom value is still corrupted via either of the instructions in the original problem report, such as :
Option 2 - corruption on creation:
- Create a JSON custom field - set "UI Editable" to "No". Set the default to JSON such as
{"key1":"value1"} - Create a prefix
- View the prefix, observe that the JSON custom field now shows an escaped string value of
"{\"key1\": \"value1\"}" - Edit the prefix and click "save" without making any changes
- Observe that the custom value is now escaped again to something like
"\"{\\\"key1\\\": \\\"value1\\\"}\"" - Repeat edit+save (without making changes) and observe that the prefix keeps getting escaped more and more as a JSON string instead of any valid JSON data structure.
@FliesLikeABrick Can you please try the updated PR, I think it should fix the problem for newly created custom fields. It won't change those that are already have incorrect data.
@arthanson @jeremystretch this PR appears to pass our tests for reproducing the issue, thank you
Understood that this does not fix existing corrupt/damaged/impacted data - I have written a tool at https://github.com/FliesLikeABrick/netbox_fix_json which can be used to repair impacted data, for anyone else who comes across this.
Any thoughts on when this will be merged? While we can ignore the human-readability aspect of it, it's becoming progressively more of a problem in some of our real-world uses. We've started having to write in bug-fix wrappers to integration code that reads these fields and recursively decode the values of all JSON fields to get the original data out. Luckily, it's not horrific to fix on the fly as you read it via API, but the longer this bug remains the more of an issue it will be in our object fields over time as people touch/edit objects. Thanks!
Any thoughts on when this will be merged? While we can ignore the human-readability aspect of it, it's becoming progressively more of a problem in some of our real-world uses. We've started having to write in bug-fix wrappers to integration code that reads these fields and recursively decode the values of all JSON fields to get the original data out. Luckily, it's not horrific to fix on the fly as you read it via API, but the longer this bug remains the more of an issue it will be in our object fields over time as people touch/edit objects. Thanks!
100% agreed, we are having to add fixes to our tooling to catch bad json and repair it; and occasionally running the tool I published to go through and fix our fields of data altogether.
@rbcollins123 @FliesLikeABrick please understand that NetBox is an extremely active project with very few people actually doing the work. For reference, we have had 73 PRs opened in the past 30 days, and each one of them consumes time.
As a reminder, you always have the option of making changes locally yourself ahead of an official release if there's a truly pressing issue.