Skip to content

hash: validate raw helper size arguments - #836

Merged
gaborcsardi merged 3 commits into
r-lib:mainfrom
fly1d:codex/hash-parameter-validation
Sep 27, 2026
Merged

gaborcsardi merged 3 commits into
r-lib:mainfrom
fly1d:codex/hash-parameter-validation

Conversation

@fly1d

@fly1d fly1d commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • apply the existing count and range checks to hash_raw_emoji() and hash_raw_animal()
  • make the object variants inherit the same validation through their raw helpers
  • add regression coverage for valid boundaries, out-of-range values, and fractional values

Fixes #834.

Verification

  • testthat::test_local(filter = "hash", reporter = "silent") - 53 passed, 0 failures, 0 errors, 0 warnings, 0 skips
  • air format R/hash.R tests/testthat/test-hash.R
  • R CMD build --no-manual
  • R CMD check --no-manual on the source tarball - Status: OK

AI assistance

OpenAI Codex (GPT-5) assisted with implementation, tests, and verification.

Fixes r-lib#834.

Assisted-by: OpenAI Codex (GPT-5)
@fly1d

fly1d commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Status note: the only failing check, format-suggest, fails before formatting because the pull_request_target workflow refuses to check out fork code for security. This is a workflow authorization boundary, not a reported formatting failure in the patch. No code change is needed from the contributor unless maintainers prefer a different trusted validation path.

Keep both the raw hash validation entry and the upstream ansi_strwrap entry.

Validation: the PR's hash implementation and regression tests are unchanged; git diff --check against upstream passes. R is unavailable in this environment, so R tests were not rerun for this synchronization.

Prepared with OpenAI Codex assistance.

@gaborcsardi gaborcsardi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

Comment thread R/hash.R
stopifnot(
is.raw(x),
is_count(size),
size >= 1 && size <= 4

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you add a comment where these values are from?

Comment thread R/hash.R
stopifnot(
is.raw(x),
is_count(n_adj),
n_adj >= 0 && n_adj <= 3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

Document that the raw helper ranges match hash_emoji() and hash_animal(), and that the upper bounds keep the shared conversions within exact double-precision integer arithmetic.

This is a comment-only follow-up to the review; executable code and regression tests are unchanged. Verified both upper bounds against the table sizes documented by the package and checked the diff for whitespace errors.

Prepared with OpenAI Codex assistance.
@gaborcsardi
gaborcsardi merged commit b4994d7 into r-lib:main Sep 27, 2026
11 checks passed
@gaborcsardi

Copy link
Copy Markdown
Member

Thank you!

@fly1d
fly1d deleted the codex/hash-parameter-validation branch September 27, 2026 16:00
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.

hash_obj_*()/hash_raw_*() variants don't validate parameter bounds, unlike their base hash_*() counterparts (affects animal and emoji hashes)

2 participants