Skip to content

#302 anchor Content-Range guard regexes with \A and \z#310

Open
edmoffo wants to merge 1 commit into
zerocracy:masterfrom
edmoffo:302-anchor-content-range-regex
Open

#302 anchor Content-Range guard regexes with \A and \z#310
edmoffo wants to merge 1 commit into
zerocracy:masterfrom
edmoffo:302-anchor-content-range-regex

Conversation

@edmoffo

@edmoffo edmoffo commented May 24, 2026

Copy link
Copy Markdown
Contributor

The total-size guard at lib/baza-rb.rb:855 read /^\*|[0-9]+$/, which because of regex precedence parsed as (^\*)|([0-9]+$) and matched every string starting with * or ending in digits. Values like *malformed, abc123, and -5 slipped past the guard, and the downstream total.to_i and e.to_i == total.to_i - 1 comparisons then either spun the chunked-download loop or terminated it on a truncated file. The neighboring range guard at :857 had the same shape with the milder ^ and $ line anchors, which also accept strings with a stray newline at the boundaries. Closes #302.

The fix replaces /^\*|[0-9]+$/ with /\A(?:\*|[0-9]+)\z/ on :855 and /^[0-9]+$/ with /\A[0-9]+\z/ on :857, so the alternation lives under one pair of string anchors and a malformed total or end value raises the existing "Total size is not valid" / "Range is not valid" errors as the guard intended. bytes 0-0/* and bytes 0-11/25 still match, so the existing test_durable_load_in_chunks and test_download_switches_host_mid_range paths are unchanged.

A regression test test_download_rejects_malformed_content_range_total in test/test_baza_edge.rb exercises *malformed, abc123, 0xff, and -5 against the download loop and pins the RuntimeError with the exact "Total size is not valid (...)" message. bundle exec rake test runs the new test green; the pre-existing single failure (test_durable_load_in_chunks) and seven pre-existing errors (the sinatra/real-HTTP harness) are unchanged by this diff. bundle exec rake rubocop reports 12 files inspected, no offenses detected.

Closes #302

…\A and \z

The total-size guard at lib/baza-rb.rb:855 read /^\*|[0-9]+$/, which
because of regex precedence parsed as (^\*)|([0-9]+$) and matched
every string starting with `*` or ending in digits. Values like
`*malformed`, `abc123`, and `-5` slipped past the guard, and the
downstream total.to_i + e.to_i comparisons then either spun the loop
or finished on a truncated file. The neighboring range guard at
lib/baza-rb.rb:857 had the same shape with the milder `^` and `$`
line anchors, which also allow a stray newline at the boundaries.

The fix replaces /^\*|[0-9]+$/ with /\A(?:\*|[0-9]+)\z/ on line 855
and /^[0-9]+$/ with /\A[0-9]+\z/ on line 857, so the alternation
lives under one pair of string anchors and a malformed total or end
value raises the existing "Total size is not valid" / "Range is not
valid" errors as the guard intended. A regression test in
test/test_baza_edge.rb exercises four malformed totals against the
download loop and pins the new behavior.

Closes zerocracy#302

@GHX5T-SOL GHX5T-SOL left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved current head 90c348c972f1435ccf0315ba095b0da099be20dc.

The fix matches #302: \A(?:\*|[0-9]+)\z keeps the */numeric alternation under one pair of string anchors, and \A[0-9]+\z prevents newline- or suffix-based acceptance for the range end. The new regression is behavior-focused and would fail on the old /^\*|[0-9]+$/ guard for cases like abc123 and *malformed.

Validation I ran locally on Ruby 3.3.11 with bundle path outside the repo:

  • bundle exec ruby -Itest -e 'ARGV << "--no-cov"; require "./test/test_baza_edge"; ARGV.delete("--no-cov")' -- --name test_download_rejects_malformed_content_range_total -> 1 test, 8 assertions, 0 failures.
  • bundle exec ruby -Itest -e 'ARGV << "--no-cov"; require "./test/test_baza_edge"; ARGV.delete("--no-cov")' -- --quiet -> 25 tests, 76 assertions, 0 failures.
  • bundle exec ruby -Itest -e 'ARGV << "--no-cov"; Dir["test/test_*.rb"].sort.each { |f| require "./#{f}" }; ARGV.delete("--no-cov")' -- --exclude /live/ --quiet -> 68 tests, 114 assertions, 0 failures.
  • bundle exec rubocop lib/baza-rb.rb test/test_baza_edge.rb -> no offenses.
  • git diff --check origin/master...HEAD -> clean.
  • git diff origin/master...HEAD | gitleaks stdin --no-banner --redact --log-level warn -> no leaks.

I also checked the hosted failure: the macOS rake job fails in TestBazaLive#test_live_full_cycle after the new test_download_rejects_malformed_content_range_total passes, so I don't see it as caused by this Content-Range guard diff. FOSSA still reports one License Compliance issue, but this PR does not add dependencies or license-bearing files.

@yegor256

Copy link
Copy Markdown
Member

Please fix the merge conflicts so this pull request can be merged.

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.

download: Content-Range total-size guard regex /^\*|[0-9]+$/ accepts any string ending in digits

3 participants