Skip to content

Check a data set name before the catalog does (#95) - #96

Merged
mgrossmann merged 3 commits into
mainfrom
fix/95-dsname-validation
Aug 8, 2026
Merged

Check a data set name before the catalog does (#95)#96
mgrossmann merged 3 commits into
mainfrom
fix/95-dsname-validation

Conversation

@mgrossmann

@mgrossmann mgrossmann commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Closes #95.

put build/ftpd.deploy.xmit sends the local path as the remote name. resolve_dsn() prepended the CWD prefix and uppercased it but checked no characters, so HERC01.BUILD/FTPD.DEPLOY.XMIT passed the RACF check, missed in LOCATE and died in SVC 99 — reported as 550 Cannot allocate dataset, blaming allocation for a name that was never allocatable.

What changed

  • ftpd_dsn_valid() (new src/ftpd#dsn.c / include/ftpd#dsn.h) checks every character before any catalog or allocation work: A-Z, 0-9, the national characters @ # $, the hyphen, . as qualifier separator with no qualifier empty, and the wildcards only where LIST and NLST resolve patterns.
  • The reply is the one z/OS gives and doc/ZOS_FTP_REFERENCE.md:1055 already documented, quoting the resolved name — what the client sent and what FTPD built are rarely the same thing:
    501 Invalid data set name "'HERC01.BUILD/FTPD.DEPLOY.XMIT'".  Use MVS Dsname conventions.
    
  • dsn_error() replaces the nine call sites that all answered the same Invalid dataset name for every way a name can fail to resolve. Wildcards keep the qualifier-naming message, an unusable character gets the new one.
  • ? is a wildcard everywhere. It was being rejected by the character check with the general message while * and % got the qualifier message FTPD_CONCEPT.md:196 documents as verified on z/OS 3.1 — for the same mistake. The set now lives in FTPD_DSN_WILDCARDS so the rejection, the message and the LIST path cannot drift apart:
    CWD MIK?  ->  501 A qualifier in "MIK?" contains an invalid character
    
    LIST is unchanged: dsn_match() implements only *, ** and %, so a pattern carrying a ? matches nothing, as before. The wildcard check stays on the raw argument rather than the resolved name — the PDS member branch of resolve_dsn() returns before validation, and RETR MEM? inside a PDS used to build a member name with a ? in it.
  • Empty qualifiers are rejected. put .profile resolved to HERC01..PROFILE and reached SVC 99 for the same reason the reported bug did. Covers a leading dot and a trailing one too; resolve_dsn() strips a trailing dot before asking, where it means prefix-only.

Deliberate non-changes

  • Members are untouched. Validation stops at the first (; sanitize_member() keeps doing the rest, and the in_pds branch of resolve_dsn() still strips a leading path — so put build/x.xmit into a PDS works exactly as before.
  • Qualifier length (≤ 8) and "must begin with a letter" stay unchecked. The length rule would turn ls ABCDEFGHIJ* — a pattern that simply matches nothing — into a 501, and MVS 3.8j is more permissive about the first character than the manuals are. Separate decisions.
  • A table searched with strchr(), not a range test. c >= 'A' && c <= 'Z' is wrong in EBCDIC: A-Z runs C1-C9/D1-D9/E2-E9, so a range test accepts the gap bytes plus \ (E0), { (C0) and } (D0). TSTDSN pins those three down; the assertion only means something in the test-mvs run.

Testing

make test-host: TSTDSN, 43 assertions, all pass (TSTADR unchanged at 29). ftpd#dsn.c is free of project and MVS headers, so the test is DUAL and also runs under make test-mvs.

The unit test covers ftpd_dsn_valid() itself. The end-to-end path — resolve_dsn() returning -2/-3, dsn_error() formatting the reply, STOR answering 501 — is verified by reading the code, not by a test; confirming it on the target is one put build/ftpd.deploy.xmit after deploy.

https://claude.ai/code/session_01Acc7qm2TkDpFjsyhwunKJk

`put build/ftpd.deploy.xmit` sends the local path as the remote name.
resolve_dsn() prepended the CWD prefix and uppercased it, but looked at no
characters at all, so HERC01.BUILD/FTPD.DEPLOY.XMIT passed the RACF check,
missed in LOCATE and finally died in SVC 99 -- reported as
"550 Cannot allocate dataset", blaming allocation for a name that was never
allocatable.

ftpd_dsn_valid() now checks every character before any catalog or allocation
work: A-Z, 0-9, the national characters @ # $ and the hyphen, with '.' as the
qualifier separator and '*'/'%' accepted only where LIST and NLST resolve
patterns. The answer is 501 with the reply z/OS gives, quoting the resolved
name, since what the client sent and what FTPD built are rarely the same.

The character set is a table searched with strchr(), not a range test: A-Z is
not contiguous in EBCDIC, so `c >= 'A' && c <= 'Z'` would accept the bytes in
the gaps plus '\', '{' and '}'.

Members are left alone -- validation stops at the first '(', and
sanitize_member() keeps doing the rest -- so PDS uploads are unaffected.
Qualifier length, empty qualifiers and the "must begin with a letter" rule
stay unchecked on purpose: the length rule would turn `ls ABCDEFGHIJ*` into a
501, and MVS 3.8j is more permissive about the first character than the
manuals are.

Nine call sites answered the same "Invalid dataset name" for every way a name
can fail to resolve; they now share dsn_error(), which says which one it was.

ftpd#dsn.c is free of project and MVS headers, so TSTDSN runs both natively
and on MVS.

Claude-Session: https://claude.ai/code/session_01Acc7qm2TkDpFjsyhwunKJk
Two names still got the wrong answer.

`CWD MIK?` was rejected by the new character check with "Invalid data set
name", while `CWD MIK*` -- the same mistake -- got the qualifier message z/OS
gives and FTPD_CONCEPT.md documents as verified. FTPD has always listed '?'
alongside '*' and '%' as a wildcard, so it is now one everywhere: the set
lives in FTPD_DSN_WILDCARDS, and the rejection, the qualifier message and the
LIST path all read it. `CWD MIK?` answers "A qualifier in "MIK?" contains an
invalid character". LIST is unchanged -- dsn_match() implements only '*', '**'
and '%', so a pattern carrying a '?' matches nothing, as before.

The wildcard check stays on the raw argument rather than the resolved name:
the PDS member branch of resolve_dsn() returns before validation, and a member
is not a pattern either. `RETR MEM?` inside a PDS used to build a member name
with a '?' in it.

`put .profile` resolved to HERC01..PROFILE and still reached SVC 99 for the
same reason the reported bug did -- nothing objected to a qualifier with
nothing in it. ftpd_dsn_valid() now requires every qualifier to hold at least
one character, which covers a leading dot and a trailing one as well.
resolve_dsn() strips a trailing dot before asking, where it means prefix-only.

Claude-Session: https://claude.ai/code/session_01Acc7qm2TkDpFjsyhwunKJk
"." is what clients send to mean "stay here", and resolving it as a name gets
the wrong answer twice.

"HLQ." + "." is "HLQ..", one trailing dot comes off again, and the empty
qualifier check added for `put .profile` would answer 501 for a no-op.

The older half is worse: resolving "." ended in the prefix-only path, which
clears the PDS context but keeps the PDS as the prefix -- so `cd .` inside a
PDS left a session where `put x` addressed the data set HLQ.PDS.X instead of
the member X.  Both are gone: "." is answered next to "..", before anything is
built, and changes nothing.

Claude-Session: https://claude.ai/code/session_01Acc7qm2TkDpFjsyhwunKJk
@mgrossmann

Copy link
Copy Markdown
Contributor Author

Follow-up in 954d22e: CWD . had to be answered before the prefix is built.

"HLQ." + "." is "HLQ..", one trailing dot comes off again, and the empty-qualifier check would have answered 501 for what clients send as a no-op.

Resolving it was already wrong for a second reason: "." landed in the prefix-only path, which clears the PDS context but keeps the PDS as the prefix — so cd . inside a PDS left a session where put x addressed the data set HLQ.PDS.X instead of the member X. "." is now answered next to ".." and changes nothing.

Checked against the existing suite while I was there: every MVS put/get in test/test_ftpd.sh passes an explicit remote name (put $BINFILE '$DSN_BIN'), so nothing there relied on a path reaching resolve_dsn().

@mgrossmann

Copy link
Copy Markdown
Contributor Author

Verified on the target (mvsdev, FTPD restarted with the new module). Every reply below is from the live session, not from reading the code.

The reported case

ftp> put build/ftpd.deploy.xmit
501 Invalid data set name "'IBMUSER.BUILD/FTPD.DEPLOY.XMIT'".  Use MVS Dsname conventions.

Wildcards keep the qualifier message, ? included

CWD MIK?  ->  501 A qualifier in "MIK?" contains an invalid character
CWD ?     ->  501 A qualifier in "?" begins with an invalid character
CWD MIK*  ->  501 A qualifier in "MIK*" contains an invalid character
CWD *     ->  501 A qualifier in "*" begins with an invalid character

Inside a PDS, where resolve_dsn() returns before validation:

CWD 'SYS1.MACLIB'  ->  250 The working directory "SYS1.MACLIB" is a partitioned data set
RETR MEM?         ->  501 A qualifier in "MEM?" contains an invalid character
RETR MEM*         ->  501 A qualifier in "MEM*" contains an invalid character
RETR DCBD         ->  125 Sending data set DCBD FIXrecfm 80

Empty qualifiers

DELE .profile   ->  501 Invalid data set name "'IBMUSER..PROFILE'".  Use MVS Dsname conventions.
MKD  ..PROFILE  ->  501 Invalid data set name "'IBMUSER...PROFILE'".  Use MVS Dsname conventions.

CWD . is a no-op, in both contexts

PWD     ->  257 "'IBMUSER.'" is working directory.
CWD .   ->  250 "IBMUSER." is the working directory name prefix.
PWD     ->  257 "'IBMUSER.'" is working directory.

CWD 'IBMUSER.T95.PDS'  ->  250 The working directory "IBMUSER.T95.PDS" is a partitioned data set
CWD .                ->  250 The working directory "IBMUSER.T95.PDS" is a partitioned data set
put VERSION MEMA     ->  125 Storing data set MEMA / 250 Transfer completed successfully.
ls                   ->  MEMA

The member store after cd . is the one that matters: PDS context survives, where it used to be silently dropped while the PDS stayed as the prefix.

Nothing else moved

put build/ftpd.deploy.xmit 'IBMUSER.T95.XMIT'  ->  125 / 250, 290720 bytes
ls 'IBMUSER.T95.*'                            ->  T95.XMIT
ls 'IBMUSER.MIK?'                             ->  empty listing, no 501 (LIST unchanged)
CWD 'SYS1.MACLIB.'                            ->  250 prefix-only
CDUP                                         ->  250 "SYS1."

Test data sets IBMUSER.T95.XMIT and IBMUSER.T95.PDS were deleted at the end; a closing ls confirms nothing is left behind.

@mgrossmann
mgrossmann merged commit 566a480 into main Aug 8, 2026
1 check passed
@mgrossmann
mgrossmann deleted the fix/95-dsname-validation branch August 8, 2026 08:15
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.

A data set name is never checked for valid characters — "put build/x.xmit" fails as "Cannot allocate"

1 participant