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
Triaged
Low
Unassigned

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
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.