Skip to content

Fix relativized readers reading a CWD-relative path instead of the store's file - #2

Merged
thorwhalen merged 1 commit into
masterfrom
fix/relative-path-key-resolution
Sep 22, 2026
Merged

thorwhalen merged 1 commit into
masterfrom
fix/relative-path-key-resolution

Conversation

@thorwhalen

Copy link
Copy Markdown
Member

Fixes the silent wrong-file read (and spurious FileNotFoundError) in
RelPathAioFileBytesReader.aget, described in
i2mint/aiofiledol#1 (part of the
umbrella tracked in i2mint/dol#83, root cause i2mint/dol#18).

mk_relative_path_store wraps by delegation, so aget is reached bound to the
INNER store but still receives the OUTER, un-transformed key. Handing that relative
key straight to AIOFile resolved it against the process CWD instead of rootdir.

New module-private _resolve_key walks the wrapper chain with dol.dig.inner_most_key
(rooted at wrapped_self(store)) to recover the real filepath. Applied to
AioFileBytesReader.aget and AioFileBytesPersister.asetitem.

Backward compatible: for an un-wrapped store, wrapped_self(store) is store and no
layer defines _id_of_key, so inner_most_key(..., default=k) returns k verbatim
— the documented bare absolute-key usage is unchanged. No public name, signature or
return type changes.

Adds the repo's first test module, including a regression guard for the bare
absolute-key path.

Branch sat pushed with green CI and no PR for two weeks
(thorwhalen/fleet_stuff cleanup). Verified: master has not moved since the branch
was cut (no rebase needed), and wads ci-local is green on py3.10 and py3.12
(11/11 checks, 7 tests passed on each).

No dependents of aiofiledol in the fleet manifest.

Closes #1

🤖 Generated with Claude Code

…ore's file

`mk_relative_path_store` wraps by delegation, so `aget` is reached bound to the
INNER store while still receiving the OUTER, un-transformed key. Handing that
relative key straight to `AIOFile` resolved it against the process CWD rather
than `rootdir`:

  - with a same-named file in the CWD, `aget` silently returned the wrong bytes;
  - without one, it raised `FileNotFoundError` for a key `list(store)` had just
    yielded.

`__getitem__ = None` makes `aget` the only read route and the validating
decorator is commented out, so nothing caught it.

New module-private `_resolve_key` walks the wrapper chain with `dol.dig`'s
`inner_most_key` (rooted at `wrapped_self(store)`) to recover the real filepath.
Applied to `AioFileBytesReader.aget` and, for the same latent case on the write
path, `AioFileBytesPersister.asetitem`.

Backward compatible: for an un-wrapped store `wrapped_self(store) is store` and
no layer defines `_id_of_key`, so `inner_most_key(..., default=k)` returns `k`
verbatim -- the bare absolute-key usage documented in the README is byte-for-byte
unchanged. No public name, signature or return type changes. The only behaviour
that changes is on the relativized classes, where it was a wrong-file read or a
spurious `FileNotFoundError`.

Adds the repo's first test module (`aiofiledol/tests/`), including a regression
guard for the bare absolute-key path.

Follow-up left out of scope to avoid a behaviour change: restoring the
commented-out `@validate_key_and_raise_key_error_on_exception` on `aget`, and
replacing `__getitem__ = None` with something that raises informatively.

Claude-Session: https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe
@thorwhalen
thorwhalen merged commit 14a2ad8 into master Sep 22, 2026
7 checks passed
@thorwhalen
thorwhalen deleted the fix/relative-path-key-resolution branch September 22, 2026 12:49
thorwhalen added a commit that referenced this pull request Sep 22, 2026
…ed (#3)

#2 resolves every aget/asetitem key through wrapped_self's wrapper chain, but
wrapped_self cannot tell a call reached through a wrapper from a direct call
on the inner store. After wrap_kvs(bare, ...), bare.aget(abs_path) went
through the wrapper's id_of_key again and read rootdir+rootdir+name. Keys the
store already accepts are now taken verbatim. Also pin dol>=0.3.50, the first
release with wrapped_self (an older installed dol made import fail).

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Key-transform bypass: aget gets the OUTER key — RelPathAioFileBytesReader.aget('a.txt') silently reads a file outside the store

1 participant