Skip to content

Remove the duplicate comma and termination handling in parse_if_match_field #2243

Description

@gaborbernat

Problem

parse_if_match_field in crates/peryx-http/src/handlers/mod.rs does two jobs twice, and the duplication makes two operators unobservable. A mutation run (#2242) found both surviving the whole suite, and no test can distinguish either.

It consumes commas in two places. The top of the loop skips them:

if field[index] == b',' {
    index += 1;
    continue;
}

and the bottom consumes one after validating it:

if field[index] != b',' {
    return Err(IfMatchError::Malformed);
}
index += 1;          // line 170

So index += 1 at line 170 can become index *= 1 and the parser still works: the comma stays put, the loop returns to the top, and the first block consumes it on the next pass.

It also terminates in two places, the loop condition and a break:

while index < field.len() {          // line 119
    while field.get(index).is_some_and(|byte| matches!(byte, b' ' | b'\t')) {
        index += 1;
    }
    if index == field.len() {
        break;
    }

index is never greater than field.len() at the top of the body, so < and <= differ only at index == field.len(), where the break fires on the extra iteration and stops. The comparison's boundary decides nothing.

Example

Both survive cargo nextest run -p peryx-http, driven directly rather than inferred:

170:15  replace += with *=   SURVIVES
119:17  replace <  with <=   SURVIVES

The same run killed 41 of 55 mutants in this file, so the suite is not weak here. These two are unobservable by construction.

Required change

Remove the duplication so each operator decides something.

  • Drop the index += 1 at line 170 and let the top of the loop consume the comma. The validation above it stays, since it is what rejects "1" "2".
  • Give the loop a single termination point so the comparison at line 119 and the break stop overlapping.

Per the campaign's rule an equivalent mutant is a code smell rather than an exemption, so the answer is the code rather than a test or an exclusion.

Acceptance criteria

  • 170:15 replace += with *= and 119:17 replace < with <= no longer appear in cargo mutants -p peryx-http --file crates/peryx-http/src/handlers/mod.rs, because the sites are gone rather than because a test covers them.
  • The rewritten operators die under mutation, confirming the survivor moved nowhere.
  • The three rstest tables over this parser pass unchanged; the parser's behaviour does not move.

Boundary

parse_if_match_field's loop structure. Do not change what the parser accepts or rejects, and do not touch the If-Match handling in grants.rs or repositories.rs.

Separately and not in scope: five index increments in this function are non-terminating under mutation, so cargo mutants books them as timeouts and gives no verdict. A structural rewrite might remove that too, but a timeout is a limit on what the tool can record rather than an untested path.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:testsConformance, perf tests, and observabilitybugSomething isn't workingpriority:P2Performance, observability, or deferred featuretype:bugIncorrect behavior or missing validation

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions