Ironic

Cannot patch keys that contain ~ or /

Bug #1604148 reported by Brad Morgan
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ironic
Fix Released
Low
Afonne-CID

Bug Description

Object attributes with keys that contain '~' or '/' cannot be operated on via the API.

https://tools.ietf.org/html/rfc6901#section-3 defines a method for escaping these characters, and this is already implemented in the python jsonpatch lib that is used, so it seems this is simply a matter of relaxing the validation regex for the 'path' attribute. (Assuming, of course, Ironic wants to support keys with the characters in them at all)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ironic (master)

Fix proposed to branch: master
Review: https://review.openstack.org/343911

Changed in ironic:
assignee: nobody → Brad Morgan (morgabra)
status: New → In Progress
Revision history for this message
Brad Morgan (morgabra) wrote :

Couple followups:

1) Does Ironic even want to support these characters in attribute keys? If no, this can be closed.

2) The tests here are probably unwanted, as they really just test the jsonpatch lib, but I left them in so you could see the use case that this fixes.

3) This is sort of a poor user experience if someone manages to get anything in the DB with these characters, even if this patch merges. (You'll need to know the magic '~0', '~1' to escape them.) What can we do about that?

Jay Faulkner (jason-oldos)
Changed in ironic:
importance: Undecided → Low
Revision history for this message
Mathieu Mitchell (mat128) wrote :

API-ref docs mention "The BODY of the PATCH request must be a JSON PATCH document, adhering to RFC 6902." and indeed, RFC 6902 mentions methods for escaping ~ and /.

Revision history for this message
Jim Rollenhagen (jim-rollenhagen) wrote :

Was asked to confirm that, yes, this is a real-world use case (we have keys with / in them downstream).

Revision history for this message
Ruby Loo (rloo) wrote :

We discussed in today's weekly ironic meeting [1], and there were no objections to supporting this. And apparently, there are folks that do have keys with '/' in them :)

[1] http://eavesdrop.openstack.org/meetings/ironic/2016/ironic.2016-09-19-17.00.log.html, starting at 17:24:33 if you can figure out the thread;)

Revision history for this message
Vladyslav Drok (vdrok) wrote :

This has not been updated for 1.5 years. setting it back to triaged

Changed in ironic:
assignee: Brad Morgan (morgabra) → nobody
status: In Progress → Triaged
Afonne-CID (cidelight)
Changed in ironic:
assignee: nobody → Afonne-CID (cidelight)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/ironic/+/933743

Changed in ironic:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ironic (master)

Reviewed: https://review.opendev.org/c/openstack/ironic/+/933743
Committed: https://opendev.org/openstack/ironic/commit/5fccd55c9f98c0d1f8e5a9a8a8996187c418eacf
Submitter: "Zuul (22348)"
Branch: master

commit 5fccd55c9f98c0d1f8e5a9a8a8996187c418eacf
Author: cid <email address hidden>
Date: Wed Oct 30 11:40:46 2024 +0100

    Allow special characters in patch field keys

    Allow special characters in patch field keys, as long as they
    are correctly encoded per JSON pointer and patch specs RFC 6901
    and RFC 6902.

    Closes-Bug: #1604148
    Change-Id: I7eeb52b51a0e8ba96103e0863819653021c79271

Changed in ironic:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/ironic 30.0.0

This issue was fixed in the openstack/ironic 30.0.0 Flamingo release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.