Skip to content

refactor(dpi/http): drop dead inner parts.len() >= 3 check#303

Merged
domcyrus merged 1 commit into
domcyrus:mainfrom
obchain:refactor/http-drop-dead-parts-len-check
May 21, 2026
Merged

refactor(dpi/http): drop dead inner parts.len() >= 3 check#303
domcyrus merged 1 commit into
domcyrus:mainfrom
obchain:refactor/http-drop-dead-parts-len-check

Conversation

@obchain
Copy link
Copy Markdown
Contributor

@obchain obchain commented May 21, 2026

Summary

analyze_http (src/network/dpi/http.rs) gates first-line parsing on parts.len() >= 3 and then re-checks the same condition inside the request branch:

if parts.len() >= 3 {
    if first_line.starts_with("HTTP/") {
        info.version = parse_http_version(parts[0]);
        info.status_code = parts[1].parse::<u16>().ok();
    } else if is_http_method(parts[0]) {
        info.method = Some(parts[0].to_string());
        info.path = Some(parts[1].to_string());
        if parts.len() >= 3 {
            info.version = parse_http_version(parts[2]);
        }
    } else {
        return None;
    }
} else {
    return None;
}

parts is not mutated between the outer guard (line 30) and the inner check (line 39), so the inner if is always true and parse_http_version(parts[2]) runs unconditionally. Drop the inner conditional and call the function directly; the expanded comment records the invariant the outer guard provides so a future reader doesn't reintroduce the check.

Behavior is unchanged.

Closes #302.

Verification

  • cargo test --lib: all pass (4 in network::dpi::http / https).
  • cargo clippy --all-targets -- -D warnings: clean.
  • cargo fmt --check: clean.

Notes

Fits the recent refactor(dpi/...) cleanup series — same shape as #296 (ssh dead conditional) and #279 (snmp unreachable arm). Net diff is 3 added / 4 removed in a single block; no test churn since both code paths through the request branch are already exercised by test_http_request.

`analyze_http` gates first-line parsing on `parts.len() >= 3` and then
re-checks the same condition inside the request branch:

    if parts.len() >= 3 {
        ...
        } else if is_http_method(parts[0]) {
            info.method = Some(parts[0].to_string());
            info.path = Some(parts[1].to_string());
            if parts.len() >= 3 {
                info.version = parse_http_version(parts[2]);
            }
        ...
    }

`parts` is not mutated between the outer and inner check, so the inner
gate is always true and `parse_http_version(parts[2])` runs
unconditionally. Drop the inner `if` and call it directly; expand the
comment to record the invariant the outer guard provides.

Behavior is unchanged.

Refs domcyrus#302
@domcyrus
Copy link
Copy Markdown
Owner

@obchain Thanks a lot for your PR, LGTM!

@domcyrus domcyrus merged commit 455878f into domcyrus:main May 21, 2026
16 checks passed
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/http): dead inner parts.len() >= 3 check in analyze_http request branch

2 participants