ankitpokhrel / ankitpokhrel/jira-cli

issue clone --replace splits on every colon and accepts empty find string (rejects URLs, corrupts body)

Aperta
#1,029 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Go
Stelle
6k
Fork
413
Merge medio
2g 3h
PR unite (30g)
1

Descrizione

## Summary

`issue clone --replace` splits the `:` pair with `strings.Split` and requires exactly 2 pieces, so any replacement value containing a colon (URLs, times, ADF text) is rejected — and a `--replace=:x` value with an empty find string sails through into `strings.ReplaceAll(s, "", x)`, which inserts `x` between every rune.

## Location

- File: `internal/cmd/issue/clone/clone.go` (clone params replace loop)
- Helpers: `ADF.ReplaceAll` (`pkg/adf/adf.go`), `strings.ReplaceAll` for wiki bodies

Relevant code path (static analysis of current `main`):

```go
pieces := strings.Split(r, ":")
if len(pieces) != 2 {
cmdutil.Fail("Invalid replace string, must be in format :. Skipping replacement...")
} else {
from, to := pieces[0], pieces[1]
...
}
```

## Problem

1. **Colons in values rejected:** `--replace 'http://old:https://new'` splits into 3+ pieces → "Invalid replace string", replacement skipped. Splitting on every colon instead of the first makes URLs and similar values inexpressible, although the documented format only promises `:`.
2. **Empty find unvalidated:** `--replace ':x'` splits into `["", "x"]` (exactly 2 pieces, passes the check) with `from == ""`. `strings.ReplaceAll(s, "", "x")` returns `x` interleaved between every character, and the ADF path does the same per text node — silent body corruption on clone.

## Trigger / Reproduction

Based on static analysis (no live Jira run performed):

1. `jira issue clone PROJ-1 --replace 'http://a:http://b'` → rejected as invalid, nothing replaced.
2. `jira issue clone PROJ-1 --replace ':x'` → `from` is `""` → summary/body get `x` injected between every rune.

Note: this is a static-analysis finding; I did not execute the CLI against a Jira instance.

## Expected Behavior

Split on the first colon only (`strings.SplitN(r, ":", 2)`), and reject empty find strings with the same "Invalid replace string" diagnostic — so values may contain colons while `""` can never become a match-everything pattern.

## Actual Behavior

Values with colons are unusable; empty find corrupts the cloned body.

## Impact

- Common replacements (URLs, timestamps, `key: value` text) cannot be expressed.
- One typo (`--replace=:x`) destroys the cloned issue body with no warning that the find pattern was empty.

## Suggested Direction

- Use `SplitN(r, ":", 2)` and add `from == ""` to the invalid branch. Both changes are confined to the replace loop; ADF/wiki paths need no change since they receive already-validated pairs.

## Evidence

- Source via API: `clone.go` replace loop as quoted; Go `strings.ReplaceAll` empty-`old` semantics are documented stdlib behavior.
- Duplicate check: issue searches for `clone replace colon` (`total_count: 0`) and `ReplaceAll replace string` (one unrelated closed parser feature) — no apparent duplicate.

## Classification

- FACT: the pair is split on all colons with no empty-find check (verified in source via API).
- INFERENCE: colon values are rejected; empty find corrupts via ReplaceAll-empty semantics.
- HYPOTHESIS: first-colon split plus empty-find rejection fixes both with no change to valid pairs.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.