ankitpokhrel / ankitpokhrel/jira-cli
issue clone --replace splits on every colon and accepts empty find string (rejects URLs, corrupts body)
- 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.