Skip to content

test(cli): cover cors required field validation#106

Draft
overtrue wants to merge 1 commit intomainfrom
codex/cors-required-field-gap
Draft

test(cli): cover cors required field validation#106
overtrue wants to merge 1 commit intomainfrom
codex/cors-required-field-gap

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

Summary

This change adds focused unit coverage for a few CORS validation branches that were introduced by the recent bucket CORS work but were still untested on main.

It specifically locks down three cases in crates/cli/src/commands/cors.rs:

  • a slash-only bucket path like local/
  • a rule with no allowed origins
  • a rule with no allowed methods

Problem

Recent CORS parser and bucket-path fixes rely on these branches to reject malformed input early, but the current test suite only covered adjacent cases such as blank values inside lists and trailing slash normalization. That left the direct empty-list and slash-only path paths unpinned.

If one of these branches regressed, users could get looser parsing for invalid CORS configs or bucket paths without a targeted test failure pointing at the exact behavior change.

Root cause

The recent feature and follow-up fixes added the validation logic, but the unit tests did not yet cover every validation branch. The suite asserted nearby malformed inputs, not the explicit empty-list and slash-only inputs handled by the parser.

Fix

Add three small unit tests that exercise the missing validation paths without changing runtime behavior:

  • reject local/ in parse_bucket_path
  • reject CORS rules with an empty allowedOrigins list
  • reject CORS rules with an empty allowedMethods list

Validation

I first ran the focused CORS unit tests:

  • cargo test -p rustfs-cli cors::tests

Then I ran the repo's CI-equivalent checks from .github/workflows/ci.yml:

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace

Note: this repository does not currently define a checked-in make pre-commit target, so I used the exact commands the CI workflow runs instead.

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.

1 participant