Skip to content

refactor(dpi/ftp): move owned Vec<u8> into String, drop redundant copy#327

Open
obchain wants to merge 1 commit into
domcyrus:mainfrom
obchain:refactor/ftp-move-vec-into-string
Open

refactor(dpi/ftp): move owned Vec<u8> into String, drop redundant copy#327
obchain wants to merge 1 commit into
domcyrus:mainfrom
obchain:refactor/ftp-move-vec-into-string

Conversation

@obchain
Copy link
Copy Markdown
Contributor

@obchain obchain commented May 26, 2026

Summary

analyze_ftp (src/network/dpi/ftp.rs) built the FTP command string from an already-owned Vec<u8> via:

let command = std::str::from_utf8(&upper).ok()?.to_string();

std::str::from_utf8 borrows the Vec's bytes and returns a &str; .to_string() then allocates a fresh String and copies those bytes in. upper is dropped on the next statement, so the copy is purely shape-driven — nothing else reads the Vec.

first_token_upper (line 169) already returns early with Vec::new() unless every byte in the token passes b.is_ascii_alphabetic(), so the bytes are valid UTF-8 by construction. Take the Vec by value with String::from_utf8 to skip the redundant copy:

let command = String::from_utf8(upper).ok()?;

The ? stays as a defensive guard against a future widening of first_token_upper's accepted byte range, and the new comment records the invariant the guard rests on.

Behavior is unchanged.

Closes #326.

Verification

  • cargo test --lib network::dpi::ftp: 13 passed, 0 failed.
  • cargo test --lib: all pass.
  • cargo clippy --all-targets -- -D warnings: clean.
  • cargo fmt --check: clean.

Notes

Fits the recent refactor(dpi/...) allocation-cleanup series (#290, #301, #307, #317) — single-line change, no behavioural impact, removes one String allocation + memcpy per FTP command request packet.

`analyze_ftp` built the FTP command string from an already-owned
`Vec<u8>` via:

    let command = std::str::from_utf8(&upper).ok()?.to_string();

`std::str::from_utf8` borrows the `Vec`'s bytes and returns a `&str`;
`.to_string()` then allocates a fresh `String` and copies those bytes
in. `upper` is dropped on the next statement, so the copy is purely
shape-driven — nothing else reads the `Vec`.

`first_token_upper` (line 169) already returns early with `Vec::new()`
unless every byte in the token passes `b.is_ascii_alphabetic()`, so
the bytes are valid UTF-8 by construction. Take the `Vec` by value
with `String::from_utf8` to skip the redundant copy. The `?` stays as
a defensive guard against a future widening of `first_token_upper`'s
accepted byte range, and the new comment records the invariant the
guard rests on.

Behavior is unchanged.

Refs domcyrus#326
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.

refactor(dpi/ftp): redundant to_string() allocation when building FTP command

1 participant