RNA: path_resolve() doesn't deal with single-quoted keys #120140

New Issue

Sybren A. Stüvel · 2024-04-01T12:15:19+02:00

Sybren A. Stüvel commented

2024-04-01 12:15:19 +02:00

System Information
Operating system: Windows-10-10.0.19045-SP0 64 Bits
Graphics card: NVIDIA GeForce RTX 4070 SUPER/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 551.61

Blender Version
Broken: version: 4.1.0, branch: blender-v4.1-release, commit date: 2024-03-25 20:42, hash: 40a5e739e270
Worked: Don't think it ever did; 3.0 and 2.78 already had this problem, and I didn't bother going back further.

Short description of error
bpy.data.path_resolve('objects["Cube"]') works, bpy.data.path_resolve("objects['Cube']") does not:

>>> bpy.data.path_resolve('objects["Cube"]')
bpy.data.objects['Cube']

>>> bpy.data.path_resolve("objects['Cube']")
Traceback (most recent call last):
  File "<blender_console>", line 1, in <module>
ValueError: BlendData.path_resolve("objects['Cube']") could not be resolved

What makes this a true bug, rather than something simply unsupported, is that repr(the_cube_object) returns a path with single quotes. So Blender produces paths that are incompatible with path_resolve().

Exact steps for others to reproduce the error

Open Blender with the factory-default scene
Switch the timeline editor to the Python console (Shift+F5)
Copy-paste the above two calls to bpy.data.path_resolve and see the results.

**System Information** Operating system: Windows-10-10.0.19045-SP0 64 Bits Graphics card: NVIDIA GeForce RTX 4070 SUPER/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 551.61 **Blender Version** Broken: version: 4.1.0, branch: blender-v4.1-release, commit date: 2024-03-25 20:42, hash: `40a5e739e270` Worked: Don't think it ever did; 3.0 and 2.78 already had this problem, and I didn't bother going back further. **Short description of error** ` bpy.data.path_resolve('objects["Cube"]')` works, ` bpy.data.path_resolve("objects['Cube']")` does not: ```python >>> bpy.data.path_resolve('objects["Cube"]') bpy.data.objects['Cube'] >>> bpy.data.path_resolve("objects['Cube']") Traceback (most recent call last): File "<blender_console>", line 1, in <module> ValueError: BlendData.path_resolve("objects['Cube']") could not be resolved ``` What makes this a true bug, rather than something simply unsupported, is that `repr(the_cube_object)` returns a path with single quotes. So Blender produces paths that are incompatible with `path_resolve()`. **Exact steps for others to reproduce the error** 1. Open Blender with the factory-default scene 2. Switch the timeline editor to the Python console (Shift+F5) 3. Copy-paste the above two calls to `bpy.data.path_resolve` and see the results.

Sybren A. Stüvel added the

labels 2024-04-01 12:15:19 +02:00

YimingWu commented

2024-04-01 12:20:44 +02:00

I believe by slightly changing rna_path_token_in_brackets and BLI_str_escape_find_quote we can support path_resolve with single bracket.

Might affect other places that uses BLI_str_quoted_substr, looks like escape characters don't play a big role there, so should be trivial...

Making a PR

I believe by slightly changing `rna_path_token_in_brackets` and `BLI_str_escape_find_quote` we can support path_resolve with single bracket. Might affect other places that uses `BLI_str_quoted_substr`, looks like escape characters don't play a big role there, so should be trivial... Making a PR

YimingWu added

and removed

labels 2024-04-01 12:22:14 +02:00

YimingWu referenced a pull request that will close this issue

2024-04-01 12:34:38 +02:00

Fix #120140: RNA: Support single quote in `path_resolve` #120141

Campbell Barton commented

2024-04-05 13:27:45 +02:00

Note that single quotes were/are intentionally not supported, even if single quotes are supported there will be subtle differences between Blender's quoting and Python's repr.

To generate blender compatible quoting there is bpy.utils.escape_identifier & unescape_identifier.

Note that single quotes were/are intentionally not supported, even if single quotes are supported there will be subtle differences between Blender's quoting and Python's repr. To generate blender compatible quoting there is `bpy.utils.escape_identifier` & `unescape_identifier`.

Campbell Barton commented

2024-04-08 07:33:34 +02:00

Looking into this further and I think this may be more an issue of documentation needing to clarify the purpose of RNA data-paths.

RNA data-paths aren't the same as printing a Python object, but this probably isn't made clear anywhere in the API docs. I think it'd be best to have a section in the API reference detailing what data-paths are exactly, and how they are expected to be used.

While supporting single quotes in isolation can work, in introduces the following issues:

The expectation that data-paths are compatible with Python strings

This isn't the case, if simple quotes are supported developers are likely to use repr() to construct animation paths only to run into rare bugs where this fails. Python will escape certain characters in ways Blender wont, making repr() fail to resolve.

To show a concrete example: assigning the names A_\x16_B or A_\uEEEE_B creates a name that wont resolve via path_resolve, I suspect there would be other similar cases, this is just two picked off hand.

Complications with Stored Data-Paths (Animation Paths & Overrides)

Having a simple data-path allows us to consider a data-path a canonical reference to a property *.

Supporting both kinds of quotes means that Python scripts will be able to construct data-paths using either kind of quote and use them to setup key-frames, drivers & overrides.

While the paths will animate (from testing !120141), the logic to check if an RNA path is animated doesn't work.
The properties don't highlight as animated in the UI for example & Inserting a key-frame causes two F-curves that reference the same property. The UI & key-framing logic could be extended to normalize data-paths before comparison or resolve them & check the target matches - but this is reasonably involved for a check which we want to run on redraw.

It also complicates versioning - currently data-paths are constructed and replaced.
BKE_animsys_fix_rna_path_rename would need to check on both single & double quoted paths, there are various other parts of the code which would need to be updated update_mapping_node_fcurve_rna_path_callback & version_liboverride_nla_strip_frame_start_end for e.g.

An alternative option to extending RNA data-paths could be to support path_resolve() having a Python compatible option which doesn't impact lower level RNA path resolution, so any valid Python data-path can work. This would end up being more like a secure eval(..).

* exceptions to this exist: there are sometimes multiple RNA paths to access the same data, it's also the case for accessing data-paths via an index which would use a name when key-framing. In general though I think these kind of cases should be avoided where possible so the chance of script authors constructing incompatible animation paths is kept to a minimum.

Looking into this further and I think this may be more an issue of documentation needing to clarify the purpose of RNA data-paths. RNA data-paths aren't the same as printing a Python object, but this probably isn't made clear anywhere in the API docs. I think it'd be best to have a section in the API reference detailing what data-paths are exactly, and how they are expected to be used. While supporting single quotes in isolation can work, in introduces the following issues: ### The expectation that data-paths are compatible with Python strings This isn't the case, if simple quotes are supported developers are likely to use `repr()` to construct animation paths only to run into rare bugs where this fails. Python will escape certain characters in ways Blender wont, making `repr()` fail to resolve. To show a concrete example: assigning the names `A_\x16_B` or `A_\uEEEE_B` creates a name that wont resolve via `path_resolve`, I suspect there would be other similar cases, this is just two picked off hand. ### Complications with Stored Data-Paths (Animation Paths & Overrides) Having a simple data-path allows us to consider a data-path a canonical reference to a property \*. Supporting both kinds of quotes means that Python scripts will be able to construct data-paths using either kind of quote and use them to setup key-frames, drivers & overrides. While the paths _will_ animate (from testing !120141), the logic to check if an RNA path is animated doesn't work. The properties don't highlight as animated in the UI for example & Inserting a key-frame causes two F-curves that reference the same property. The UI & key-framing logic could be extended to normalize data-paths before comparison or resolve them & check the target matches - but this is reasonably involved for a check which we want to run on redraw. It also complicates versioning - currently data-paths are constructed and replaced. `BKE_animsys_fix_rna_path_rename` would need to check on both single & double quoted paths, there are various other parts of the code which would need to be updated `update_mapping_node_fcurve_rna_path_callback` & `version_liboverride_nla_strip_frame_start_end` for e.g. --- An alternative option to extending RNA data-paths could be to support `path_resolve()` having a Python compatible option which doesn't impact lower level RNA path resolution, so any valid Python data-path can work. This would end up being more like a secure `eval(..)`. ---- \* exceptions to this exist: there are sometimes multiple RNA paths to access the same data, it's also the case for accessing data-paths via an index which would use a name when key-framing. In general though I think these kind of cases should be avoided where possible so the chance of script authors constructing incompatible animation paths is kept to a minimum.

👍 1

Campbell Barton referenced this issue

2024-04-08 07:41:10 +02:00

Fix #120140: RNA: Support single quote in `path_resolve` #120141

Sybren A. Stüvel commented

2024-04-08 12:10:13 +02:00

Thanks Campbell for your detailed response.

RNA data-paths aren't the same as printing a Python object, but this probably isn't made clear anywhere in the API docs. I think it'd be best to have a section in the API reference detailing what data-paths are exactly, and how they are expected to be used.

I think this is a good, and necessary, idea.

A few things are getting mixed here.

There should be one canonical RNA path for a property: This I completely agree with. Having multiple RNA paths for a property makes so many things more cumbersome, it's not worth the effort of changing this. At least it's not what I wanted to suggest with this report ;-)

Not all Python expressions are valid RNA paths: This is quite strict, in the sense that it translates to "any string that looks like an RNA path that is not the canonical RNA path is invalid". I'm not sure how useful this strictness is for lookups by RNA path. As I said above, I can totally see some parts of Blender being restricted to the canonical path, but for me that doesn't immediately translate to such strictness for a function like struct.path_resolve().

My actual point is neither of the above though. I've done some more digging, and the confusion is caused by this: the result of repr(rna_struct) is a mixture of RNA path and Python expression. For a string bpy.data.armatures['Armature.001'].bones["Bone"], the bpy.data.armatures['Armature.001'] part is generated as a Python expression, whereas the bones["Bone"] part is an RNA path produced by rna_Bone_path().

For the curious (and my own reference later on), this is implemented in pyrna_struct_repr(BPy_StructRNA *self) in bpy_rna.cc. The use of %R in PyUnicode_FromFormat("bpy.data.%s[%R].%s", ...) is the culprit here. IMO this should use the same quoting rules as the rest of the RNA path handling code.

The expectation that data-paths are compatible with Python strings: This isn't the case

When I interpret this strictly & literally, then it reduces the result of repr(rna_struct) to some human-readable string that is useless for anything else, as it is the concatenation of two incompatible strings.

if simple quotes are supported developers are likely to use repr() to construct animation paths only to run into rare bugs where this fails. Python will escape certain characters in ways Blender wont, making repr() fail to resolve.

To show a concrete example: assigning the names A_\x16_B or A_\uEEEE_B creates a name that wont resolve via path_resolve, I suspect there would be other similar cases, this is just two picked off hand.

Much of that works fine though. It's only when you use repr() on the name itself, and then feed that to path_resolve(), where things go wrong:

>>> C.active_pose_bone.name = 'A_\x16_B'
>>> eval(repr(C.active_pose_bone))
bpy.data.objects['Armature.001'].pose.bones["A_⑯_B"]
>>> D.objects['Armature.001'].path_resolve(f'pose.bones["{C.active_pose_bone.name}"]')
bpy.data.objects['Armature.001'].pose.bones["A_⑯_B"]
>>> D.objects['Armature.001'].path_resolve(f'pose.bones[{C.active_pose_bone.name!r}]')
Traceback (most recent call last):
  File "<blender_console>", line 1, in <module>
ValueError: Object.path_resolve("pose.bones['A_\x16_B']") could not be resolved

Note that in the above, I've replaced the actual \x16 codepoint with ⑯ for displayability in Gitea.

An alternative option to extending RNA data-paths could be to support path_resolve() having a Python compatible option which doesn't impact lower level RNA path resolution, so any valid Python data-path can work. This would end up being more like a secure eval(..).

I think this could even be the default behaviour for path_resolve(). To me, A.path_resolve(B) succeeding does not mean that B is the canonical RNA path. To get that I'd like to use repr(A.path_resolve(B)), except that doesn't return an RNA path at all. And posebone.path_from_id() excludes the owning object, making it useless for my actual purpose (using the bone's RNA path to get to its owner).

To me, these are different functions:

Resolve this RNA path → should accept a wide range of paths to resolve
Convert this RNA path to the canonical path
Is the property with this RNA path animated? → fine if it only works for the canonical path
Insert an animation key for this RNA path → also fine if this only (reliably) works for the canonical path

If one of these is actually used in place of another (like using the fact that it resolves as an indication that the path is the canonical one), that seems like a deeper design mistake to me.

Thanks Campbell for your detailed response. > RNA data-paths aren't the same as printing a Python object, but this probably isn't made clear anywhere in the API docs. I think it'd be best to have a section in the API reference detailing what data-paths are exactly, and how they are expected to be used. I think this is a good, and necessary, idea. A few things are getting mixed here. **There should be one canonical RNA path for a property:** This I completely agree with. Having multiple RNA paths for a property makes so many things more cumbersome, it's not worth the effort of changing this. At least it's not what I wanted to suggest with this report ;-) **Not all Python expressions are valid RNA paths:** This is quite strict, in the sense that it translates to "any string that looks like an RNA path that is not the canonical RNA path is invalid". I'm not sure how useful this strictness is for lookups by RNA path. As I said above, I can totally see some parts of Blender being restricted to the canonical path, but for me that doesn't immediately translate to such strictness for a function like `struct.path_resolve()`. My actual point is neither of the above though. I've done some more digging, and the confusion is caused by this: **the result of `repr(rna_struct)` is a mixture of RNA path and Python expression**. For a string `bpy.data.armatures['Armature.001'].bones["Bone"]`, the `bpy.data.armatures['Armature.001']` part is generated as a Python expression, whereas the `bones["Bone"]` part is an RNA path produced by `rna_Bone_path()`. For the curious (and my own reference later on), this is implemented in `pyrna_struct_repr(BPy_StructRNA *self)` in `bpy_rna.cc`. The use of `%R` in `PyUnicode_FromFormat("bpy.data.%s[%R].%s", ...)` is the culprit here. IMO this should use the same quoting rules as the rest of the RNA path handling code. > The expectation that data-paths are compatible with Python strings: This isn't the case When I interpret this strictly & literally, then it reduces the result of `repr(rna_struct)` to some human-readable string that is useless for anything else, as it is the concatenation of two incompatible strings. > if simple quotes are supported developers are likely to use `repr()` to construct animation paths only to run into rare bugs where this fails. Python will escape certain characters in ways Blender wont, making `repr()` fail to resolve. > > To show a concrete example: assigning the names `A_\x16_B` or `A_\uEEEE_B` creates a name that wont resolve via `path_resolve`, I suspect there would be other similar cases, this is just two picked off hand. Much of that works fine though. It's only when you use `repr()` on the name itself, and then feed that to `path_resolve()`, where things go wrong: ```python >>> C.active_pose_bone.name = 'A_\x16_B' >>> eval(repr(C.active_pose_bone)) bpy.data.objects['Armature.001'].pose.bones["A_⑯_B"] >>> D.objects['Armature.001'].path_resolve(f'pose.bones["{C.active_pose_bone.name}"]') bpy.data.objects['Armature.001'].pose.bones["A_⑯_B"] >>> D.objects['Armature.001'].path_resolve(f'pose.bones[{C.active_pose_bone.name!r}]') Traceback (most recent call last): File "<blender_console>", line 1, in <module> ValueError: Object.path_resolve("pose.bones['A_\x16_B']") could not be resolved ``` Note that in the above, I've replaced the actual `\x16` codepoint with ⑯ for displayability in Gitea. > An alternative option to extending RNA data-paths could be to support `path_resolve()` having a Python compatible option which doesn't impact lower level RNA path resolution, so any valid Python data-path can work. This would end up being more like a secure `eval(..)`. I think this could even be the default behaviour for `path_resolve()`. To me, `A.path_resolve(B)` succeeding does _not_ mean that `B` is the canonical RNA path. To get that I'd like to use `repr(A.path_resolve(B))`, except that doesn't return an RNA path at all. And `posebone.path_from_id()` excludes the owning object, making it useless for my actual purpose (using the bone's RNA path to get to its owner). To me, these are different functions: - Resolve this RNA path → should accept a wide range of paths to resolve - Convert this RNA path to the canonical path - Is the property with this RNA path animated? → fine if it only works for the canonical path - Insert an animation key for this RNA path → also fine if this only (reliably) works for the canonical path If one of these is actually used in place of another (like using the fact that it resolves as an indication that the path is the canonical one), that seems like a deeper design mistake to me.

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

RNA: path_resolve() doesn't deal with single-quoted keys #120140

The expectation that data-paths are compatible with Python strings

Complications with Stored Data-Paths (Animation Paths & Overrides)