support full-length UTF-8 path components - #313
Conversation
🤖 Augment PR SummarySummary: This PR removes fixed AFP URL-path storage in favor of owned dynamic paths.
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
augment review |
|
I may be confused, and I may be thinking about ZFS instead of Netatalk, but somewhere n the back of my head I'm thinking that it's not 255 but 511 for some reason. Also it's not clear exactly which IPC interface this is referring too? Do you mean between FUSE and afpc? |
|
@andylemin the 255 character limit for file paths originates from AFP, e.g. https://afp.fyi/docs/programming_guide/concepts/#designating-a-path-to-a-cnode please let me know if I'm overlooking a crucial fact here and the semantics must be adjusted! the IPC in question is that between the controller daemons and the client libraries, e.g. afpsld to a client that implements libafpsl. but on second thought I might remove this version check after all -- since we're a pre-release project without an install base it feels like over engineering, even if it will be important to introduce eventually |
|
@rdmark found it; openzfs/zfs#13043 It should be 512 bytes to support 255 Unicode-16 characters plus null terminator. Notice this is the case for macOS too. So might need to do some digging into this if the spec says 255 characters and we are only providing 256 bytes, while still supporting Unicode-16? Ok, I understand now, so the IPC is just between the userland and the daemon 👍 Which makes sense as we use FUSE library so there is no IPC in the data path (good). I don't see anything wrong with the idea of adding robust handling of long paths provided at the userland CLI interfaces. I guess I'm still waiting to see real world use cases built around the CLI tools. Naively, personally I only expect to use them to setup a mount, and that's probably it. I'm looking forward to being surprised. CLI Finder search would be useful. |
Replace the fixed-size AFP URL path and IPC filepath buffers with owned, dynamically allocated storage. Encode IPC paths as length-tagged variable-size payloads, and validate both request and response bounds so valid long paths are neither truncated nor misparsed. Add URL lifecycle helpers and update callers to manage dynamic paths safely. Apply AFP 3.x filename limits by Unicode character count (up to 255 per component) while retaining byte-based limits for older protocols and encodings. Add an IPC hello handshake for the Stateless library. A stateless client and afpsld daemon now verify the protocol magic and major version before processing requests; minor-version differences remain compatible. Add test coverage for long UTF-8 names and IPC namespaces.
|
@andylemin note that we have a 255 character limit on (AFP) paths, but a 65,535 byte buffer for the (joint) paths before this PR we used a static "scratch buffer" of 768 bytes for every AFP path, used in hundreds of places, and replacing this with the dynamic buffer is what made this refactor this huge. also note that since this refactor touches the core of the libafpclient library, so the FUSE client benefits from the improved reliability as well -- and so do clients that implement the AFP Stateless library, such as kio-afp regarding the usefulness of CLI tools: I have found real world use for the batch copy mode of afpcmd, analogous to how scp works |
|
|
Great. Thanks for confirming. LGTM |
2 participants
Replace the fixed-size AFP URL path and IPC filepath buffers with owned, dynamically allocated storage. Encode IPC paths as length-tagged variable-size payloads, and validate both request and response bounds so valid long paths are neither truncated nor misparsed.
Add URL lifecycle helpers and update callers to manage dynamic paths safely. Apply AFP 3.x filename limits by Unicode character count (up to 255 per component) while retaining byte-based limits for older protocols and encodings.
Add an IPC hello handshake for the Stateless library. The stateless client and afpsld daemon now verify the protocol magic and major version before processing requests; minor-version differences remain compatible.
Add test coverage for long UTF-8 names and IPC namespaces.