ankitpokhrel / ankitpokhrel/jira-cli

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

Offen
#1,029 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Go
Sterne
6k
Forks
413
Ø Merge
2 T. 3 Std.
Gemergte PRs (30 T.)
1

Beschreibung

## 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.

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.