#366: Add pname format and length validation to lock/unlock/name_exists?/recent#376
Conversation
|
CI failure note: The All 118 other tests pass, and all non-live checks (rubocop, copyrights, markdown-lint, pdd, reuse, xcop, actionlint, typos, yamllint) pass successfully. |
07a6734 to
706842c
Compare
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Reviewed current head 706842c2966cb825c093f0064c791a823d197e5e; this looks correct to me.
The change adds the same /\A[a-z0-9-]+\z/ and length <= 32 guards before each touched method appends the name into the URL. The new test/test_baza_edge.rb cases use fake_baza, which is actually a real BazaRb pointed at example.org, so they exercise the production client path.
I also checked the missing overlong variants with a local no-network WebMock probe: invalid and overlong values for lock, unlock, name_exists?, and recent all raised before any HTTP request, while valid valid-name reached the expected stubbed lock, unlock, exists, and recent paths.
Local validation passed: bundle check; syntax checks for lib/baza-rb.rb and test/test_baza_edge.rb; git diff --check upstream/master...HEAD; focused new edge tests; full coverage-disabled test/test_baza_edge.rb, test/test_baza.rb, and test/test_fake.rb; bundle exec rubocop; bundle exec rake picks; and bundle exec rake yard.
Hosted readback: all non-rake checks pass. Ubuntu rake passes the new validation tests and the existing touched client tests, then fails only TestBazaLive#test_live_full_cycle with a Zerocracy live job not finishing; macOS rake was canceled. I did not run full local bundle exec rake because it includes live Zerocracy API tests.
|
@GHX5T-SOL Thanks for the review! You've earned +10 points for this contribution: +18 as a base reward for authoring the code review, then -8 deducted due to absolutely no comments being posted during the review process. Your running score is +1404; don't forget to check your Zerocracy account too. |
…bug, improve name uniqueness
…name_exists?/recent
|
@yegor256 this PR is blocked by the All our PRs have been rebased on the test fix from #393 (increased timeout, unique job names, fixed elapsed time display). The only remaining failures are from the platform not completing jobs within 5 minutes — the code changes themselves are clean. A review and platform fix would unblock all of these. Thanks! |
|
@yegor256 fixed it, please review |
|
@VasilevNStas Thanks for the contribution! You've earned +24 points. Please, keep them coming. Your running score is +2148; don't forget to check your Zerocracy account too). |
Problem
BazaRb::Fake#checknamevalidates pname format (\A[a-z0-9-]+\z) and length (≤32), but four BazaRb methods only checked nil and empty:lock(pname, owner)— pname goes into/lock/<pname>unlock(pname, owner)— pname goes into/unlock/<pname>name_exists?(pname)— pname goes into/exists/<pname>recent(name)— name goes into/recent/<name>.txtAll four risk URL injection and Fake/Real divergence.
Solution
Added format and length guards to each method after the existing nil/empty checks, matching
Fake#checkname:Tests added (5 new edge tests)
test_lock_raises_when_pname_is_invalidtest_lock_raises_when_pname_is_too_longtest_unlock_raises_when_pname_is_invalidtest_name_exists_raises_when_pname_is_invalidtest_recent_raises_when_pname_is_invalidVerification
bundle exec rubocop— 0 offensesCloses #366.