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

Open
opened 2024-04-01 12:15:19 +02:00 by Sybren A. Stüvel · 4 comments

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

  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.
**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
Status
Needs Triage
Priority
Normal
Type
Report
labels 2024-04-01 12:15:19 +02:00
Member

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
Module
Core
Status
Confirmed
and removed
Status
Needs Triage
labels 2024-04-01 12:22:14 +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`.

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

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
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser Project (Legacy)
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#120140
No description provided.