Repository navigation
fix: reject reserved node names on creation - #4484
Open
barlowa124 wants to merge 2 commits into
Open
barlowa124 wants to merge 2 commits into
barlowa124 wants to merge 2 commits into
Conversation
The Zarr format reserves node names beginning with '__' and the name
'zarr.json'. create_group('__a') wrote the reserved name, and
create_group('a/zarr.json') wrote a directory that shadows the parent's
metadata document, orphaning the whole subtree from members(). Both are
now refused with MetadataValidationError before anything is stored.
The check validates each '/'-separated segment in AsyncGroup.create_group,
AsyncGroup.create_array, and the module-level create_array/create_group
'name'/'path' arguments. Empty segments pass through because leading and
repeated slashes have long been normalized away by path joining.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4484 +/- ##
=======================================
Coverage 94.69% 94.69%
=======================================
Files 94 94
Lines 13606 13617 +11
=======================================
+ Hits 12884 12895 +11
Misses 722 722
🚀 New features to boost your workflow:
|
4 of 7 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Covers item 1 of #4453. The spec reserves node names beginning with
__and the namezarr.json, yet creation accepted both:create_group("__a")wrote the reserved name outright, andcreate_group("a/zarr.json")produceda/zarr.json/zarr.json. Thatdirectory shadows the group's own metadata document and leaves the
subtree unreachable through
members().A
_validate_node_namehelper inzarr.core.commonrejects any/segment that iszarr.json,__-prefixed,.or.., raisingMetadataValidationErrorbefore anything is stored. It runs insideAsyncGroup.create_group,AsyncGroup.create_arrayand themodule-level
create_array/create_groupname/path arguments, so therequire_*and sync wrappers are covered through delegation.Two boundaries are deliberate: empty segments still pass through
(
store_path /has always normalized leading and repeated slashes),and the
open_*read paths are untouched so existing stores holdingsuch names still open.
create_nodeskeeps its documentedcaller-responsibility contract.
For reviewers
Worth a second look. Whether
open_group/open_arrayin createmodes should share the check is open (mode
"a"is also a read mode,so it would block opening stores that already contain such nodes).
Whether rejecting
./..segments is welcome is open too, as iswhether the check belongs in Zarr format 2 at all, where
zarr.jsonis not a metadata name though a directory so named still orphans its
parent. The check is one
loop and narrows easily. Locally each rejected name raised before any
write and
members()stayed consistent.tests/test_group.pyisgreen including the leading-slash parametrization.
Author attestation
TODO
Done:
docs/user-guide/*.mdchanges/CI: