Skip to content

remove_label skips the canonicalization add_label applies, and reports removed:true whether or not anything was deleted #344

Description

@beardthelion

add_label (api/labels.rs:24) canonicalizes before storing:

let label = req.label.trim().to_lowercase();

remove_label (api/labels.rs:56-70) passes the raw path segment straight through:

Path((owner, name, label)): Path<(String, String, String)>,
...
state.db.remove_label(&record.id, &label).await?;
Ok(Json(serde_json::json!({ "label": label, "removed": true })))

So creating Bug stores bug, and DELETE /api/v1/repos/{owner}/{repo}/labels/Bug matches nothing.

The second half is the part worth fixing carefully. db::remove_label (db/mod.rs:1928-1935) is
Result<()> and discards the row count:

sqlx::query("DELETE FROM repo_labels WHERE repo_id = $1 AND label = $2")
    .bind(repo_id).bind(label).execute(&self.pool).await?;
Ok(())

The handler then returns 200 with {"removed": true} unconditionally. A delete that removed nothing
is indistinguishable from one that worked, which is the no-op-rendered-as-success shape we have been
trying to stamp out on the client side.

Both handlers are correctly owner-gated (require_repo_owner), so this is not an authorization issue.

Fix direction

Extract the trim-and-lowercase plus the charset validation into a shared helper in api/labels.rs and
call it from both handlers. Have db::remove_label return the affected row count, and let the handler
report removed from it (or 404 when nothing matched).

Verified by reading both handlers and the query at origin/main 50d3cbb. Not driven end to end.

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:nodegitlawb-node — the serving node and REST APIkind:bugDefect fix — wrong or unsafe behaviorsev:lowCosmetic, cleanup, or nice-to-havesubsystem:apiNode REST API request/response surface

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions