Skip to content

fix(q): reject oversized credential strings - #13

Merged
protocolstardust merged 1 commit into
RayforceDB:masterfrom
belowzeroff:fix/q-strict-arg-validation
Sep 25, 2026
Merged

protocolstardust merged 1 commit into
RayforceDB:masterfrom
belowzeroff:fix/q-strict-arg-validation

Conversation

@belowzeroff

@belowzeroff belowzeroff commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

What changes for users

.q.connect now rejects user or password strings longer than 127 bytes with a length error instead of silently truncating them and later reporting a misleading authentication failure.

Valid credentials and all existing valid .q.* calls keep their behavior.

Why

The credential buffers are 128 bytes including the terminating NUL. Previously, oversized credentials were truncated before the handshake, which could make a valid user input fail with an unrelated auth error.

Validation

  • Rebased onto fresh master (92e0d91)
  • make test: codec, CLI, server, malformed-frame, real-q interop, all client suites, poll, and push tests passed
  • make test client regression includes a 128-byte username and expects length
  • git diff --check: passed

This PR intentionally contains only the length validation and its regression test; the overlapping type, timeout, and handle checks are already in PR #10.

@protocolstardust

Copy link
Copy Markdown
Collaborator

Not merged as is: this overlaps #10, which landed on master today (e08ab96) and already covers most of it:

  • bool / temporal atoms are no longer accepted as integers,
  • user / password must be strings,
  • timeout is validated (type and 0..INT_MAX),
  • .q.send / .q.close require a non-negative i64 handle,
  • closing an unknown handle is a handle error.

The branch now conflicts in embed/rayforce_q.c, and the new 01_connection.rfl lines duplicate what 06_errors.rfl / 07_auth.rfl assert since #10.

What is still new here and worth keeping

  • A length error for a user/password longer than 127 bytes instead of silent truncation (which today ends in a confusing auth failure). This is a real improvement.

What I would not take over #10

Please rebase onto master and reduce the PR to the length check plus one test line, then it can go in quickly. Leaving this open for that.

@belowzeroff
belowzeroff force-pushed the fix/q-strict-arg-validation branch from 99f8fae to 8288945 Compare September 25, 2026 15:36
@belowzeroff belowzeroff changed the title fix(q): validate q builtin argument types fix(q): reject oversized credential strings Sep 25, 2026
@belowzeroff

Copy link
Copy Markdown
Contributor Author

Addressed:

  • rebased onto current master
  • removed the overlapping type, timeout, and handle changes already covered by fix(q): validate handles and wire protocol frames #10
  • kept only the length error for user/password longer than 127 bytes
  • reduced the regression coverage to one client test line

make test passes, including the new oversized-username case.

@protocolstardust
protocolstardust merged commit cdbdecb into RayforceDB:master Sep 25, 2026
1 check passed
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.

2 participants