Skip to content

Avoid mutating caller components in FTP.build - #251

Open
OskarEichler wants to merge 2 commits into
ruby:masterfrom
OskarEichler:codex/ftpbuild-uri
Open

Avoid mutating caller components in FTP.build#251
OskarEichler wants to merge 2 commits into
ruby:masterfrom
OskarEichler:codex/ftpbuild-uri

Conversation

@OskarEichler

Copy link
Copy Markdown

Summary

Normalize the FTP path after make_components_hash has copied the input. Do not replace entries in the caller Hash or Array before copying.

Reproduction

Build twice from {host: 'example.test', path: '/folder'}. Before, the first build changes the caller path to '/%2Ffolder' and the next build encodes it again. Frozen Hash/Array inputs raise FrozenError. After, repeated builds are equivalent and input components remain unchanged.

Verification

  • Ruby 4.0.6 through rbenv; existing bundle exec rake test: 93 tests / 3,039 assertions / zero failures or errors, both baseline and this isolated patch.
  • 72 checks cover Hash/Array and frozen/mutable containers, empty/relative/absolute paths and absent/a/i typecodes.
  • Syntax and git diff --check pass. Supplemental Lint adds no findings against the 31-finding baseline.
  • No test/spec files added or modified, per this contribution's explicit no-new-tests constraint. Focused reproductions ran externally.

Compatibility and limitations

Intentional ownership correction: FTP.build no longer mutates caller components. Generated URIs for the initial input remain the same. No public API removal, dependency change or Ruby minimum change.

Based on master 696df43b0be4c05d3a0dcf7db582126c915d860d. Other Ruby/OS runtimes were not executed locally. No production traffic or live mail/FTP services were used. Existing related PRs/issues were checked; this is a focused contribution, not exhaustive behavioral coverage.

@olleolleolle olleolleolle 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.

The change looks reasonable, and allows the caller to own what they passed in to the method.

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.

2 participants