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

UI corrupts read-only JSON fields #16640

Open
FliesLikeABrick opened this issue Jun 17, 2024 · 6 comments · May be fixed by #16713
Open

UI corrupts read-only JSON fields #16640

FliesLikeABrick opened this issue Jun 17, 2024 · 6 comments · May be fixed by #16713
Assignees
Labels
severity: medium Results in substantial degraded or broken functionality for specfic workflows status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@FliesLikeABrick
Copy link

FliesLikeABrick commented Jun 17, 2024

Deployment Type

Self-hosted

NetBox Version

v4.0.5

Python Version

3.11

Steps to Reproduce

This issue can be reproduced in a couple different workflows. This was originally reported in #16328 after we saw this behavior, which we believe was introduced between 3.6.3 and 3.7.7. We have now reproduced this in a clean install of 4.0.5 running inside docker using netbox-docker 2.9.1.

Edits through the API are not impacted.

Option 1 - corruption during edit:

  1. Create a JSON custom field - set "UI Editable" to "No". Leave the default value as null.
  2. Create a prefix, it is not necessary to populate the JSON field with a value -- it will default to null
  3. View the prefix, observe that the JSON custom field shows a value of "-" in the UI due to it being null
  4. Edit the prefix and click "save" without making any changes
  5. Observe that the custom value now changes to the quoted string "null"

Option 2 - corruption on creation:

  1. Create a JSON custom field - set "UI Editable" to "No". Set the default to JSON such as {"key1":"value1"}
  2. Create a prefix
  3. View the prefix, observe that the JSON custom field now shows an escaped string value of "{\"key1\": \"value1\"}"
  4. Edit the prefix and click "save" without making any changes
  5. Observe that the custom value now changes to the quoted string "null"

Expected Behavior

The JSON value should not be changed due to the UI being set to read-only for this field

Observed Behavior

The server is setting a new and incorrect/corrupt value for this field in the database. The native JSON is being converted to an escaped string

@FliesLikeABrick FliesLikeABrick added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels Jun 17, 2024
@FliesLikeABrick
Copy link
Author

We have stepped through releases and found that this was introduced in 3.7.7, the issue was not present in 3.7.6. Both of the "steps to reproduce" workflows described above are triggering the bug in 3.7.7.

@FliesLikeABrick FliesLikeABrick changed the title editing prefix in UI results in read-only JSON fields being converted to string UI corrupts read-only JSON fields Jun 18, 2024
@FliesLikeABrick
Copy link
Author

"editing" and saving a prefix multiple times in the UI (not actually editing anything, let alone the read-only JSON field) results in the field value being escaped multiple times:
image

@FliesLikeABrick
Copy link
Author

We have written a script that finds and repairs the affected data, which can be found at https://github.com/FliesLikeABrick/netbox_fix_json

@jeffgdotorg
Copy link
Collaborator

Thanks for reporting this problem and for providing clear and comprehensive steps to reproduce. I was able to reproduce the option-1 case exactly as described in a fresh 4.0.5 install. The option-2 case reproduces slightly differently from how it's written up in the issue body (the value { "thing1": "value1" } gets escaped and displays as "\"{\\\"thing1\\\": \\\"value1\\\"}\""), but I see your follow-on comment lines up with that. I suggest editing your issue body to match what you (and I) actually see in option 2, but in any case I'm moving your issue out of triage.

Extra thanks for doing the additional archaeology to determine where the problem seems to have been introduced. That's potentially a big help.

@jeffgdotorg jeffgdotorg removed their assignment Jun 21, 2024
@jeffgdotorg jeffgdotorg added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation severity: medium Results in substantial degraded or broken functionality for specfic workflows and removed status: needs triage This issue is awaiting triage by a maintainer labels Jun 21, 2024
@FliesLikeABrick
Copy link
Author

Huh, my option2 text was actually correct but github or markdown was eating the backslashes . I have edited it to be closer to what is being observed but my escaping may not 100% match the values seen in a reproduction (especially depending on whether someone is observing the raw database value vs that returned in the UI vs that returned by the API before or after being decoded as JSON)

@arthanson arthanson self-assigned this Jun 24, 2024
@arthanson arthanson added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jun 24, 2024
@arthanson arthanson linked a pull request Jun 24, 2024 that will close this issue
@FliesLikeABrick
Copy link
Author

FliesLikeABrick commented Jun 26, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity: medium Results in substantial degraded or broken functionality for specfic workflows status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants