Skip to content

test: cover public protocol boundary limits - #7

Open
slegarraga wants to merge 2 commits into
rufatixx:mainfrom
slegarraga:tests/protocol-limits
Open

test: cover public protocol boundary limits#7
slegarraga wants to merge 2 commits into
rufatixx:mainfrom
slegarraga:tests/protocol-limits

Conversation

@slegarraga

Copy link
Copy Markdown

What & why

Adds deterministic boundary tests for the public protocol limits and validation paths: payload size, node ID length, content type length, TTL, empty identifiers, and ciphertext validation through decryption.

Closes #2.

Coverage

  • Payload exactly at 256 KiB accepted; one byte above rejected.
  • Node ID exactly at max length accepted; one character above rejected.
  • Content type exactly at max length accepted; one character above rejected.
  • TTL at the seven-day maximum accepted; zero and above maximum rejected.
  • Empty required identifiers rejected.
  • Decryption rejects a packet whose ciphertext exceeds the payload limit.

Verification

  • dotnet test tests/RelayOS.Core.Tests: 37 tests pass.
  • The local machine has SDK 10.0.107 while the repo pins 10.0.201 in global.json; I temporarily bypassed the pin for local verification and did not modify or include global.json.
  • Test-only change.

Signed-off-by: Sebastian Legarraga <64795732+slegarraga@users.noreply.github.com>
@slegarraga
slegarraga requested a review from rufatixx as a code owner August 6, 2026 23:58

@rufatixx rufatixx left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused, test-only contribution. I verified that the current patch is safe to run, and CI is green on the repository-pinned .NET 10.0.201 SDK (37/37 tests, simulator, dependency audit, and package build all pass).

Before merging, please complete the inbound-validation part of #2. Most current boundary assertions exercise only RelayIdentity or RelayCryptography.Encrypt; RelayPacketValidator is reached through Decrypt only for oversized ciphertext.

Please extend the tests so a valid packet is also accepted through Decrypt at the maximum payload, node-ID, content-type, and TTL boundaries, and so mutated inbound packets are rejected for the listed invalid/empty node-ID, content-type, and TTL cases (including both sender and recipient identifiers where applicable). Keep the tests deterministic and test-only.

Once those public ingress paths are covered and CI remains green, #2 will be fully satisfied and this will be ready to merge.

Signed-off-by: Sebastian Legarraga <64795732+slegarraga@users.noreply.github.com>
@slegarraga

Copy link
Copy Markdown
Author

Thanks for the detailed review. I extended the test suite so inbound packets are exercised through Decrypt: max payload, node-ID, content-type, and TTL are accepted at their boundaries, and empty/whitespace/oversized sender and recipient IDs, empty/oversized content types, plus zero and oversized TTL, are rejected on the public ingress path. CI will validate on the repository-pinned SDK.

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.

Add boundary tests for public protocol limits

2 participants