Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

Bug: jj backend passes paths as fileset expressions, corrupting or breaking diffs for punctuated filenames

Abierto
  • ¿Algo no está bien?
#341 2 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
78/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
go
Área
devtools

Línea de trabajo

Empieza en app/diff/jj.go leyendo FileDiff y totalOldLines, centrándote en los dos argumentos de ruta que se pasan después de --. Añade cobertura de regresión que compruebe la jj argv para rutas que contienen signos de puntuación, en lugar de comprobar únicamente que se devuelve un diff. Se considera terminado cuando ambas llamadas afectadas apuntan de forma segura a un único archivo literal, mientras jjblame.go permanece sin cambios.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Summary

(*Jj).FileDiff appends the file path after -- on the jj diff command line. jj parses those arguments as fileset expressions — a query language with globs, set operators and quoting — not as literal paths. A filename only works if it happens to be a valid bare fileset string.

Any filename containing punctuation is therefore at risk, in one of two ways. Of 31 filenames tested through (*Jj).FileDiff, 23 produced a wrong result:

  • 19 fail loudly. Characters such as $, (, :, # don't parse. The diff pane shows error loading diff and the file can't be reviewed.
  • 5 fail silently. * and ? are globs; &, |, ~ are set operators. jj exits 0, and revdiff renders a diff for the wrong set of files — either empty, or other files' diffs concatenated into this one. Nothing indicates a problem.

The silent class is the more serious one: it shows wrong content under a correct-looking filename, and annotations made there are exported against line numbers that don't exist.

Repro — loud failure

jj git init
printf 'one\ntwo\n' > '$test.txt'
jj describe -m base && jj new
printf 'one\ntwoX\n' > '$test.txt'
revdiff

The file lists in the tree normally (jj diff --summary takes no path arg). Selecting it shows:

error loading diff: get file diff for $test.txt: jj diff --git --context=1000000 -- $test.txt:
Error: Failed to parse fileset: Syntax error
  --> 1:1
  |
1 | $test.txt
  | ^---
  = expected <strict_identifier>, <bare_string>, or <expression>

Works when quoted: jj diff --git -- 'root-file:"$test.txt"'

Repro — silent failure

Two files, where one name globs onto the other:

jj git init
printf 'one\n' > 'a*b.txt'; printf 'one\n' > 'axb.txt'
jj describe -m base && jj new
printf 'two\n' > 'a*b.txt'; printf 'two\n' > 'axb.txt'

FileDiff{Path: "a*b.txt"} returns no error and 8 rows for what is a one-line change:

[0] type="-" old=1 new=0 "one"
[1] type="+" old=0 new=1 "two"
[2] type=" " old=2 new=2 "diff --git a/axb.txt b/axb.txt"     <- other file's header as context
[3] type=" " old=3 new=3 "index 5626abf0f7..f719efd430 100644"
[4] type="-" old=4 new=0 "-- a/axb.txt"                        <- marker mangled into a removal
[5] type="+" old=0 new=4 "++ b/axb.txt"
[6] type="-" old=1 new=0 "one"                                 <- numbering restarts
[7] type="+" old=0 new=1 "two"

axb.txt's content is rendered inside a*b.txt's diff. Because parseUnifiedDiff expects a single-file diff, the embedded diff --git/index headers are absorbed as context lines and the ---/+++ markers become fake add/remove rows with the first character eaten. Line numbers restart per embedded file.

This scales with the number of matches — in a 31-file test repo the same call returned 184 rows.

Annotations key on path + line number, and both are wrong here, so an annotation placed on one of these rows is written to -o output against a line that doesn't exist. That makes this a correctness bug in exported review output, not only a display bug.

Root cause

Two call sites, both app/diff/jj.go:

Site Call Behavior
jj.go:149 (*Jj).FileDiff — append(args, "--", req.Path) the failures above
jj.go:193 (*Jj).totalOldLines — jj file show ... -- <file> silent; error discarded, returns 0, so compact mode drops the trailing ⋯ N lines ⋯ divider

git and hg take literal pathspecs; jj replaced that with the fileset language. fileset appears nowhere in the codebase — the semantics were never accounted for.

Full character results

31 filenames of the form a<char>b.txt, each driven through (*Jj).FileDiff:

Outcome n Characters
Silent glob 2 * ?
Silent empty 3 & | ~
Parse error 19 ! " # $ % ' ( ) , : ; < = > [ ^ ` { }
Correct 8 + - . @ ] _ ␣ (incl. plain baseline; ] is safe, [ is not)

Representative ASCII punctuation, not an exhaustive byte sweep.

Not affected

  • Blame — jj file annotate (jjblame.go:24) takes a genuine path. Raw name works; cwd-file:"…" fails there (No such path). Must not be changed.
  • --all-files — jj file list (directory.go:51) takes no path args.
  • git — verified with $ok.txt and :top.txt.
  • hg — hg.go:155 passes literal paths (hg needs explicit glob:/re:). Reasoned from code only — hg not installed, not executed.

Suggested fix

Quote the path as a file pattern at the two affected sites only:

// jj parses post-`--` args as fileset expressions, so $ ! ( ) : , break the
// parse and * ? & | ~ silently resolve to the wrong set of files.
func jjFilesetPath(path string) string {
	esc := strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(path)
	return `root-file:"` + esc + `"`
}

Escaping verified for a"b.txt → root-file:"a\"b.txt" and a\b.txt → root-file:"a\\b.txt".

  • Prefer root-file: over cwd-file: — NewJj gets vcsRoot (renderer_setup.go:53) so they're equivalent today, but root-file: survives a workDir change.
  • Do not apply to jjblame.go.
  • Worth adding separately: reject input with more than one diff --git header in parseUnifiedDiff. That's the layer that turned a bad path argument into plausible-looking rows.

Tests

No jj test uses a path with punctuation. A regression test should assert the argv passed to jj — asserting "a diff came back" would pass for a*b.txt, which currently returns wrong rows with no error.

Environment

revdiff master e3eb732 · jj 0.44.0-af45d57 · go 1.27.0 linux/amd64

Lenguaje dominante
Go
Estrellas
896
Forks
92
Merge medio
17 h 49 min
PR fusionados (30 d)
17

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de umputun/revdiff

Todos los issues de umputun/revdiff

Issues similares

Más issues de Go

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.