refactor(domain): share the path list bounds between the git adapters
PathList.Add skips the paths that are not addressable and stops at the path and byte bounds, setting Truncated. The gitcli and ermitegit walkers use it, so the rule lives in one place, and the hexcheck ignore of the ermitegit walker is no longer needed. ermitegit checks a path before it writes it to a chunk and no longer allocates a string for each directory prefix. ParseTreePath and IsAddressablePath are tested against one table.
- commit
- b5e1b0c85819656a3e48e246d2f6af41c5b32bc4
- author
- brice · · 2026-10-12 05:05 UTC
- parent
- 9494523
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +136 -54
M internal/adapters/ermitegit/convert.go +21 -31
| @@ -337,21 +337,17 @@ type pathWalker struct { | |||
| 337 | 337 | maxPaths int | |
| 338 | 338 | maxBytes int | |
| 339 | 339 | ||
| 340 | - | // paths are the paths kept. Their bytes live in chunks: a chunk is filled | |
| 340 | + | // list collects the paths. Their bytes live in chunks: a chunk is filled | |
| 341 | 341 | // by a strings.Builder, so a path is read back as a string without a copy, | |
| 342 | 342 | // and a full chunk is left to the paths that point into it instead of | |
| 343 | 343 | // being grown and copied. | |
| 344 | - | paths []string | |
| 345 | - | chunk strings.Builder | |
| 346 | - | prefix []byte // "dir/sub/" of the directory being read | |
| 347 | - | size int // bytes of the paths kept | |
| 348 | - | truncated bool | |
| 349 | - | bad error | |
| 344 | + | list domain.PathList | |
| 345 | + | chunk strings.Builder | |
| 346 | + | prefix []byte // "dir/sub/" of the directory being read | |
| 347 | + | bad error | |
| 350 | 348 | } | |
| 351 | 349 | ||
| 352 | 350 | // listPaths lists the files and symbolic links of a revision recursively. | |
| 353 | - | // | |
| 354 | - | //hexcheck:ignore suspicious-business-logic-in-adapter converts ermite-git values into domain values; the limits and the orders come from domain | |
| 355 | 351 | func listPaths(ctx context.Context, r *git.Repo, rev domain.Revision, maxPaths, maxBytes int) (domain.PathList, error) { | |
| 356 | 352 | commit, root, err := commitTree(r, rev) | |
| 357 | 353 | if err != nil { | |
| @@ -362,12 +358,11 @@ func listPaths(ctx context.Context, r *git.Repo, rev domain.Revision, maxPaths, | |||
| 362 | 358 | return domain.PathList{}, err | |
| 363 | 359 | } | |
| 364 | 360 | w := &pathWalker{ctx: ctx, r: r, maxPaths: maxPaths, maxBytes: maxBytes} | |
| 361 | + | w.list.CommitID, w.list.TreeID = commit, treeID | |
| 365 | 362 | if err := w.walk(root); err != nil { | |
| 366 | 363 | return domain.PathList{}, err | |
| 367 | 364 | } | |
| 368 | - | list := domain.PathList{CommitID: commit, TreeID: treeID, Truncated: w.truncated} | |
| 369 | - | list.Paths = w.paths | |
| 370 | - | return list, nil | |
| 365 | + | return w.list, nil | |
| 371 | 366 | } | |
| 372 | 367 | ||
| 373 | 368 | // walk lists one directory. The recursion is inside the callback of ReadTree, | |
| @@ -388,7 +383,7 @@ func (w *pathWalker) walk(tree git.ObjectID) error { | |||
| 388 | 383 | w.prefix = append(w.prefix, e.Name...) | |
| 389 | 384 | // The paths below a directory that is not addressable are not | |
| 390 | 385 | // either: this also bounds the depth of the recursion. | |
| 391 | - | if !domain.IsAddressablePath(string(w.prefix)) { | |
| 386 | + | if !domain.IsAddressablePathBytes(w.prefix) { | |
| 392 | 387 | w.prefix = w.prefix[:mark] | |
| 393 | 388 | return true | |
| 394 | 389 | } | |
| @@ -399,7 +394,7 @@ func (w *pathWalker) walk(tree git.ObjectID) error { | |||
| 399 | 394 | w.bad = err | |
| 400 | 395 | return false | |
| 401 | 396 | } | |
| 402 | - | return !w.truncated | |
| 397 | + | return !w.list.Truncated | |
| 403 | 398 | case e.Mode.IsRegular(), e.Mode.IsSymlink(): | |
| 404 | 399 | return w.add(e.Name) | |
| 405 | 400 | } | |
| @@ -413,27 +408,22 @@ func (w *pathWalker) walk(tree git.ObjectID) error { | |||
| 413 | 408 | } | |
| 414 | 409 | ||
| 415 | 410 | // add lists the file name of the current directory, and says whether the walk | |
| 416 | - | // goes on. | |
| 411 | + | // goes on. A path that is not addressable is not written to a chunk. | |
| 417 | 412 | func (w *pathWalker) add(name []byte) bool { | |
| 418 | - | n := len(w.prefix) + len(name) | |
| 419 | - | if w.chunk.Cap()-w.chunk.Len() < n { | |
| 413 | + | mark := len(w.prefix) | |
| 414 | + | w.prefix = append(w.prefix, name...) | |
| 415 | + | path := w.prefix | |
| 416 | + | w.prefix = w.prefix[:mark] | |
| 417 | + | if !domain.IsAddressablePathBytes(path) { | |
| 418 | + | return true | |
| 419 | + | } | |
| 420 | + | if w.chunk.Cap()-w.chunk.Len() < len(path) { | |
| 420 | 421 | w.chunk = strings.Builder{} | |
| 421 | - | w.chunk.Grow(max(pathChunk, n)) | |
| 422 | + | w.chunk.Grow(max(pathChunk, len(path))) | |
| 422 | 423 | } | |
| 423 | 424 | start := w.chunk.Len() | |
| 424 | - | w.chunk.Write(w.prefix) | |
| 425 | - | w.chunk.Write(name) | |
| 426 | - | path := w.chunk.String()[start:] | |
| 427 | - | switch { | |
| 428 | - | case !domain.IsAddressablePath(path): | |
| 429 | - | case len(w.paths) >= w.maxPaths || w.size+n > w.maxBytes: | |
| 430 | - | w.truncated = true | |
| 431 | - | return false | |
| 432 | - | default: | |
| 433 | - | w.size += n | |
| 434 | - | w.paths = append(w.paths, path) | |
| 435 | - | } | |
| 436 | - | return true | |
| 425 | + | w.chunk.Write(path) | |
| 426 | + | return w.list.Add(w.chunk.String()[start:], w.maxPaths, w.maxBytes) | |
| 437 | 427 | } | |
| 438 | 428 | ||
| 439 | 429 | // fillLastCommits sets the last commit of the first MaxLastCommitEntries | |
| @@ -337,21 +337,17 @@ type pathWalker struct { | |||||
| 337 | maxPaths int | 337 | maxPaths int | ||
| 338 | maxBytes int | 338 | maxBytes int | ||
| 339 | 339 | ||||
| 340 | - | // paths are the paths kept. Their bytes live in chunks: a chunk is filled | 340 | + | // list collects the paths. Their bytes live in chunks: a chunk is filled |
| 341 | // by a strings.Builder, so a path is read back as a string without a copy, | 341 | // by a strings.Builder, so a path is read back as a string without a copy, | ||
| 342 | // and a full chunk is left to the paths that point into it instead of | 342 | // and a full chunk is left to the paths that point into it instead of | ||
| 343 | // being grown and copied. | 343 | // being grown and copied. | ||
| 344 | - | paths []string | 344 | + | list domain.PathList |
| 345 | - | chunk strings.Builder | 345 | + | chunk strings.Builder |
| 346 | - | prefix []byte // "dir/sub/" of the directory being read | 346 | + | prefix []byte // "dir/sub/" of the directory being read |
| 347 | - | size int // bytes of the paths kept | 347 | + | bad error |
| 348 | - | truncated bool | |||
| 349 | - | bad error | |||
| 350 | } | 348 | } | ||
| 351 | 349 | ||||
| 352 | // listPaths lists the files and symbolic links of a revision recursively. | 350 | // listPaths lists the files and symbolic links of a revision recursively. | ||
| 353 | - | // | |||
| 354 | - | //hexcheck:ignore suspicious-business-logic-in-adapter converts ermite-git values into domain values; the limits and the orders come from domain | |||
| 355 | func listPaths(ctx context.Context, r *git.Repo, rev domain.Revision, maxPaths, maxBytes int) (domain.PathList, error) { | 351 | func listPaths(ctx context.Context, r *git.Repo, rev domain.Revision, maxPaths, maxBytes int) (domain.PathList, error) { | ||
| 356 | commit, root, err := commitTree(r, rev) | 352 | commit, root, err := commitTree(r, rev) | ||
| 357 | if err != nil { | 353 | if err != nil { | ||
| @@ -362,12 +358,11 @@ func listPaths(ctx context.Context, r *git.Repo, rev domain.Revision, maxPaths, | |||||
| 362 | return domain.PathList{}, err | 358 | return domain.PathList{}, err | ||
| 363 | } | 359 | } | ||
| 364 | w := &pathWalker{ctx: ctx, r: r, maxPaths: maxPaths, maxBytes: maxBytes} | 360 | w := &pathWalker{ctx: ctx, r: r, maxPaths: maxPaths, maxBytes: maxBytes} | ||
| 361 | + | w.list.CommitID, w.list.TreeID = commit, treeID | |||
| 365 | if err := w.walk(root); err != nil { | 362 | if err := w.walk(root); err != nil { | ||
| 366 | return domain.PathList{}, err | 363 | return domain.PathList{}, err | ||
| 367 | } | 364 | } | ||
| 368 | - | list := domain.PathList{CommitID: commit, TreeID: treeID, Truncated: w.truncated} | 365 | + | return w.list, nil |
| 369 | - | list.Paths = w.paths | |||
| 370 | - | return list, nil | |||
| 371 | } | 366 | } | ||
| 372 | 367 | ||||
| 373 | // walk lists one directory. The recursion is inside the callback of ReadTree, | 368 | // walk lists one directory. The recursion is inside the callback of ReadTree, | ||
| @@ -388,7 +383,7 @@ func (w *pathWalker) walk(tree git.ObjectID) error { | |||||
| 388 | w.prefix = append(w.prefix, e.Name...) | 383 | w.prefix = append(w.prefix, e.Name...) | ||
| 389 | // The paths below a directory that is not addressable are not | 384 | // The paths below a directory that is not addressable are not | ||
| 390 | // either: this also bounds the depth of the recursion. | 385 | // either: this also bounds the depth of the recursion. | ||
| 391 | - | if !domain.IsAddressablePath(string(w.prefix)) { | 386 | + | if !domain.IsAddressablePathBytes(w.prefix) { |
| 392 | w.prefix = w.prefix[:mark] | 387 | w.prefix = w.prefix[:mark] | ||
| 393 | return true | 388 | return true | ||
| 394 | } | 389 | } | ||
| @@ -399,7 +394,7 @@ func (w *pathWalker) walk(tree git.ObjectID) error { | |||||
| 399 | w.bad = err | 394 | w.bad = err | ||
| 400 | return false | 395 | return false | ||
| 401 | } | 396 | } | ||
| 402 | - | return !w.truncated | 397 | + | return !w.list.Truncated |
| 403 | case e.Mode.IsRegular(), e.Mode.IsSymlink(): | 398 | case e.Mode.IsRegular(), e.Mode.IsSymlink(): | ||
| 404 | return w.add(e.Name) | 399 | return w.add(e.Name) | ||
| 405 | } | 400 | } | ||
| @@ -413,27 +408,22 @@ func (w *pathWalker) walk(tree git.ObjectID) error { | |||||
| 413 | } | 408 | } | ||
| 414 | 409 | ||||
| 415 | // add lists the file name of the current directory, and says whether the walk | 410 | // add lists the file name of the current directory, and says whether the walk | ||
| 416 | - | // goes on. | 411 | + | // goes on. A path that is not addressable is not written to a chunk. |
| 417 | func (w *pathWalker) add(name []byte) bool { | 412 | func (w *pathWalker) add(name []byte) bool { | ||
| 418 | - | n := len(w.prefix) + len(name) | 413 | + | mark := len(w.prefix) |
| 419 | - | if w.chunk.Cap()-w.chunk.Len() < n { | 414 | + | w.prefix = append(w.prefix, name...) |
| 415 | + | path := w.prefix | |||
| 416 | + | w.prefix = w.prefix[:mark] | |||
| 417 | + | if !domain.IsAddressablePathBytes(path) { | |||
| 418 | + | return true | |||
| 419 | + | } | |||
| 420 | + | if w.chunk.Cap()-w.chunk.Len() < len(path) { | |||
| 420 | w.chunk = strings.Builder{} | 421 | w.chunk = strings.Builder{} | ||
| 421 | - | w.chunk.Grow(max(pathChunk, n)) | 422 | + | w.chunk.Grow(max(pathChunk, len(path))) |
| 422 | } | 423 | } | ||
| 423 | start := w.chunk.Len() | 424 | start := w.chunk.Len() | ||
| 424 | - | w.chunk.Write(w.prefix) | 425 | + | w.chunk.Write(path) |
| 425 | - | w.chunk.Write(name) | 426 | + | return w.list.Add(w.chunk.String()[start:], w.maxPaths, w.maxBytes) |
| 426 | - | path := w.chunk.String()[start:] | |||
| 427 | - | switch { | |||
| 428 | - | case !domain.IsAddressablePath(path): | |||
| 429 | - | case len(w.paths) >= w.maxPaths || w.size+n > w.maxBytes: | |||
| 430 | - | w.truncated = true | |||
| 431 | - | return false | |||
| 432 | - | default: | |||
| 433 | - | w.size += n | |||
| 434 | - | w.paths = append(w.paths, path) | |||
| 435 | - | } | |||
| 436 | - | return true | |||
| 437 | } | 427 | } | ||
| 438 | 428 | ||||
| 439 | // fillLastCommits sets the last commit of the first MaxLastCommitEntries | 429 | // fillLastCommits sets the last commit of the first MaxLastCommitEntries | ||
M internal/adapters/gitcli/read.go +3 -7
| @@ -306,7 +306,6 @@ func (g *Git) ListPaths(ctx context.Context, repo domain.RepoName, rev domain.Re | |||
| 306 | 306 | return domain.PathList{}, fmt.Errorf("paths of %q: %w", repo, err) | |
| 307 | 307 | } | |
| 308 | 308 | list := domain.PathList{CommitID: obj.commit, TreeID: treeID} | |
| 309 | - | size := 0 | |
| 310 | 309 | err = g.pipe(ctx, readArgs(dir, "ls-tree", "-r", "-z", treeID.String()), func(r *bufio.Reader) error { | |
| 311 | 310 | for { | |
| 312 | 311 | tok, err := readToken(r) | |
| @@ -322,16 +321,13 @@ func (g *Git) ListPaths(ctx context.Context, repo domain.RepoName, rev domain.Re | |||
| 322 | 321 | } | |
| 323 | 322 | // meta is "<mode> <type> <id>": a submodule is a commit. | |
| 324 | 323 | _, rest, _ := strings.Cut(meta, " ") | |
| 325 | - | if typ, _, _ := strings.Cut(rest, " "); typ == "commit" || !domain.IsAddressablePath(path) { | |
| 324 | + | if typ, _, _ := strings.Cut(rest, " "); typ == "commit" { | |
| 326 | 325 | continue | |
| 327 | 326 | } | |
| 328 | - | if len(list.Paths) >= maxPaths || size+len(path) > maxBytes { | |
| 329 | - | list.Truncated = true | |
| 327 | + | // A copy: path would keep the whole "<mode> <type> <id>\t" entry alive. | |
| 328 | + | if !list.Add(strings.Clone(path), maxPaths, maxBytes) { | |
| 330 | 329 | return errStop | |
| 331 | 330 | } | |
| 332 | - | size += len(path) | |
| 333 | - | // A copy: path would keep the whole "<mode> <type> <id>\t" entry alive. | |
| 334 | - | list.Paths = append(list.Paths, strings.Clone(path)) | |
| 335 | 331 | } | |
| 336 | 332 | }) | |
| 337 | 333 | if err != nil { | |
| @@ -306,7 +306,6 @@ func (g *Git) ListPaths(ctx context.Context, repo domain.RepoName, rev domain.Re | |||||
| 306 | return domain.PathList{}, fmt.Errorf("paths of %q: %w", repo, err) | 306 | return domain.PathList{}, fmt.Errorf("paths of %q: %w", repo, err) | ||
| 307 | } | 307 | } | ||
| 308 | list := domain.PathList{CommitID: obj.commit, TreeID: treeID} | 308 | list := domain.PathList{CommitID: obj.commit, TreeID: treeID} | ||
| 309 | - | size := 0 | |||
| 310 | err = g.pipe(ctx, readArgs(dir, "ls-tree", "-r", "-z", treeID.String()), func(r *bufio.Reader) error { | 309 | err = g.pipe(ctx, readArgs(dir, "ls-tree", "-r", "-z", treeID.String()), func(r *bufio.Reader) error { | ||
| 311 | for { | 310 | for { | ||
| 312 | tok, err := readToken(r) | 311 | tok, err := readToken(r) | ||
| @@ -322,16 +321,13 @@ func (g *Git) ListPaths(ctx context.Context, repo domain.RepoName, rev domain.Re | |||||
| 322 | } | 321 | } | ||
| 323 | // meta is "<mode> <type> <id>": a submodule is a commit. | 322 | // meta is "<mode> <type> <id>": a submodule is a commit. | ||
| 324 | _, rest, _ := strings.Cut(meta, " ") | 323 | _, rest, _ := strings.Cut(meta, " ") | ||
| 325 | - | if typ, _, _ := strings.Cut(rest, " "); typ == "commit" || !domain.IsAddressablePath(path) { | 324 | + | if typ, _, _ := strings.Cut(rest, " "); typ == "commit" { |
| 326 | continue | 325 | continue | ||
| 327 | } | 326 | } | ||
| 328 | - | if len(list.Paths) >= maxPaths || size+len(path) > maxBytes { | 327 | + | // A copy: path would keep the whole "<mode> <type> <id>\t" entry alive. |
| 329 | - | list.Truncated = true | 328 | + | if !list.Add(strings.Clone(path), maxPaths, maxBytes) { |
| 330 | return errStop | 329 | return errStop | ||
| 331 | } | 330 | } | ||
| 332 | - | size += len(path) | |||
| 333 | - | // A copy: path would keep the whole "<mode> <type> <id>\t" entry alive. | |||
| 334 | - | list.Paths = append(list.Paths, strings.Clone(path)) | |||
| 335 | } | 331 | } | ||
| 336 | }) | 332 | }) | ||
| 337 | if err != nil { | 333 | if err != nil { | ||
M internal/domain/git.go +43 -6
| @@ -167,20 +167,37 @@ func IsAddressablePath(s string) bool { | |||
| 167 | 167 | return s != "" && treePathProblem(s) == "" | |
| 168 | 168 | } | |
| 169 | 169 | ||
| 170 | + | // IsAddressablePathBytes is IsAddressablePath for a path held in bytes; it | |
| 171 | + | // spares the copy into a string. | |
| 172 | + | func IsAddressablePathBytes(b []byte) bool { | |
| 173 | + | return len(b) > 0 && len(b) <= maxTreePathLen && utf8.Valid(b) && pathShapeProblem(b) == "" | |
| 174 | + | } | |
| 175 | + | ||
| 170 | 176 | // treePathProblem returns why s cannot be a non-root tree path, or "". | |
| 171 | 177 | func treePathProblem(s string) string { | |
| 172 | 178 | if len(s) > maxTreePathLen || !utf8.ValidString(s) { | |
| 173 | 179 | return "too long or not valid UTF-8" | |
| 174 | 180 | } | |
| 175 | - | for _, c := range s { | |
| 176 | - | if c < 0x20 || c == 0x7f { | |
| 177 | - | return "control character" | |
| 181 | + | return pathShapeProblem(s) | |
| 182 | + | } | |
| 183 | + | ||
| 184 | + | // pathShapeProblem checks the control characters and the components of a path | |
| 185 | + | // that is valid UTF-8 and short enough. The bytes of a multi-byte character | |
| 186 | + | // are all 0x80 or more, so it looks at bytes. | |
| 187 | + | func pathShapeProblem[S ~string | ~[]byte](s S) string { | |
| 188 | + | start := 0 | |
| 189 | + | for i := 0; i <= len(s); i++ { | |
| 190 | + | if i < len(s) && s[i] != '/' { | |
| 191 | + | if c := s[i]; c < 0x20 || c == 0x7f { | |
| 192 | + | return "control character" | |
| 193 | + | } | |
| 194 | + | continue | |
| 178 | 195 | } | |
| 179 | - | } | |
| 180 | - | for part := range strings.SplitSeq(s, "/") { | |
| 181 | - | if part == "" || part == "." || part == ".." { | |
| 196 | + | n := i - start | |
| 197 | + | if n == 0 || (n == 1 && s[start] == '.') || (n == 2 && s[start] == '.' && s[start+1] == '.') { | |
| 182 | 198 | return "empty, . or .. component" | |
| 183 | 199 | } | |
| 200 | + | start = i + 1 | |
| 184 | 201 | } | |
| 185 | 202 | return "" | |
| 186 | 203 | } | |
| @@ -356,6 +373,26 @@ type PathList struct { | |||
| 356 | 373 | Paths []string | |
| 357 | 374 | // Truncated is true when more paths exist than the ones in Paths. | |
| 358 | 375 | Truncated bool | |
| 376 | + | ||
| 377 | + | size int // bytes of Paths, kept by Add | |
| 378 | + | } | |
| 379 | + | ||
| 380 | + | // Add lists p, the next path of a walk in tree order, within the bounds of | |
| 381 | + | // ListPaths: at most maxPaths paths and maxBytes bytes of paths. A path that | |
| 382 | + | // is not addressable (IsAddressablePath) is left out. When a bound is reached | |
| 383 | + | // p is not listed, Truncated is set, and Add returns false: the walk can stop. | |
| 384 | + | // Otherwise it returns true. The bytes of p are kept, not copied. | |
| 385 | + | func (l *PathList) Add(p string, maxPaths, maxBytes int) bool { | |
| 386 | + | if !IsAddressablePath(p) { | |
| 387 | + | return true | |
| 388 | + | } | |
| 389 | + | if len(l.Paths) >= maxPaths || l.size+len(p) > maxBytes { | |
| 390 | + | l.Truncated = true | |
| 391 | + | return false | |
| 392 | + | } | |
| 393 | + | l.size += len(p) | |
| 394 | + | l.Paths = append(l.Paths, p) | |
| 395 | + | return true | |
| 359 | 396 | } | |
| 360 | 397 | ||
| 361 | 398 | // MaxLastCommitEntries bounds the entries of one directory that get a last | |
| @@ -167,20 +167,37 @@ func IsAddressablePath(s string) bool { | |||||
| 167 | return s != "" && treePathProblem(s) == "" | 167 | return s != "" && treePathProblem(s) == "" | ||
| 168 | } | 168 | } | ||
| 169 | 169 | ||||
| 170 | + | // IsAddressablePathBytes is IsAddressablePath for a path held in bytes; it | |||
| 171 | + | // spares the copy into a string. | |||
| 172 | + | func IsAddressablePathBytes(b []byte) bool { | |||
| 173 | + | return len(b) > 0 && len(b) <= maxTreePathLen && utf8.Valid(b) && pathShapeProblem(b) == "" | |||
| 174 | + | } | |||
| 175 | + | ||||
| 170 | // treePathProblem returns why s cannot be a non-root tree path, or "". | 176 | // treePathProblem returns why s cannot be a non-root tree path, or "". | ||
| 171 | func treePathProblem(s string) string { | 177 | func treePathProblem(s string) string { | ||
| 172 | if len(s) > maxTreePathLen || !utf8.ValidString(s) { | 178 | if len(s) > maxTreePathLen || !utf8.ValidString(s) { | ||
| 173 | return "too long or not valid UTF-8" | 179 | return "too long or not valid UTF-8" | ||
| 174 | } | 180 | } | ||
| 175 | - | for _, c := range s { | 181 | + | return pathShapeProblem(s) |
| 176 | - | if c < 0x20 || c == 0x7f { | 182 | + | } |
| 177 | - | return "control character" | 183 | + | |
| 184 | + | // pathShapeProblem checks the control characters and the components of a path | |||
| 185 | + | // that is valid UTF-8 and short enough. The bytes of a multi-byte character | |||
| 186 | + | // are all 0x80 or more, so it looks at bytes. | |||
| 187 | + | func pathShapeProblem[S ~string | ~[]byte](s S) string { | |||
| 188 | + | start := 0 | |||
| 189 | + | for i := 0; i <= len(s); i++ { | |||
| 190 | + | if i < len(s) && s[i] != '/' { | |||
| 191 | + | if c := s[i]; c < 0x20 || c == 0x7f { | |||
| 192 | + | return "control character" | |||
| 193 | + | } | |||
| 194 | + | continue | |||
| 178 | } | 195 | } | ||
| 179 | - | } | 196 | + | n := i - start |
| 180 | - | for part := range strings.SplitSeq(s, "/") { | 197 | + | if n == 0 || (n == 1 && s[start] == '.') || (n == 2 && s[start] == '.' && s[start+1] == '.') { |
| 181 | - | if part == "" || part == "." || part == ".." { | |||
| 182 | return "empty, . or .. component" | 198 | return "empty, . or .. component" | ||
| 183 | } | 199 | } | ||
| 200 | + | start = i + 1 | |||
| 184 | } | 201 | } | ||
| 185 | return "" | 202 | return "" | ||
| 186 | } | 203 | } | ||
| @@ -356,6 +373,26 @@ type PathList struct { | |||||
| 356 | Paths []string | 373 | Paths []string | ||
| 357 | // Truncated is true when more paths exist than the ones in Paths. | 374 | // Truncated is true when more paths exist than the ones in Paths. | ||
| 358 | Truncated bool | 375 | Truncated bool | ||
| 376 | + | ||||
| 377 | + | size int // bytes of Paths, kept by Add | |||
| 378 | + | } | |||
| 379 | + | ||||
| 380 | + | // Add lists p, the next path of a walk in tree order, within the bounds of | |||
| 381 | + | // ListPaths: at most maxPaths paths and maxBytes bytes of paths. A path that | |||
| 382 | + | // is not addressable (IsAddressablePath) is left out. When a bound is reached | |||
| 383 | + | // p is not listed, Truncated is set, and Add returns false: the walk can stop. | |||
| 384 | + | // Otherwise it returns true. The bytes of p are kept, not copied. | |||
| 385 | + | func (l *PathList) Add(p string, maxPaths, maxBytes int) bool { | |||
| 386 | + | if !IsAddressablePath(p) { | |||
| 387 | + | return true | |||
| 388 | + | } | |||
| 389 | + | if len(l.Paths) >= maxPaths || l.size+len(p) > maxBytes { | |||
| 390 | + | l.Truncated = true | |||
| 391 | + | return false | |||
| 392 | + | } | |||
| 393 | + | l.size += len(p) | |||
| 394 | + | l.Paths = append(l.Paths, p) | |||
| 395 | + | return true | |||
| 359 | } | 396 | } | ||
| 360 | 397 | ||||
| 361 | // MaxLastCommitEntries bounds the entries of one directory that get a last | 398 | // MaxLastCommitEntries bounds the entries of one directory that get a last | ||
M internal/domain/git_test.go +69 -10
| @@ -73,21 +73,80 @@ func TestParseTreePath(t *testing.T) { | |||
| 73 | 73 | require.NoError(t, err) | |
| 74 | 74 | assert.Equal(t, "a/b", p.String()) | |
| 75 | 75 | assert.Equal(t, "a/b/c", p.Join("c")) | |
| 76 | + | } | |
| 76 | 77 | ||
| 77 | - | for _, bad := range []string{"/a", "a//b", "./a", "a/..", "..", ".", "a\x00b", "a\nb", "\xff", strings.Repeat("a", 4097), "//"} { | |
| 78 | - | _, err := domain.ParseTreePath(bad) | |
| 79 | - | assert.ErrorIs(t, err, domain.ErrInvalidPath, "%q", bad) | |
| 78 | + | // ParseTreePath and IsAddressablePath share one rule. The only differences | |
| 79 | + | // are the root ("" is a valid path to parse, and not addressable) and the | |
| 80 | + | // trailing slash (ParseTreePath drops one, IsAddressablePath wants the path | |
| 81 | + | // as it is). | |
| 82 | + | func TestTreePathRule(t *testing.T) { | |
| 83 | + | long := strings.Repeat("a", 4096) | |
| 84 | + | for _, tt := range []struct { | |
| 85 | + | path string | |
| 86 | + | ok bool // parses, and is addressable | |
| 87 | + | }{ | |
| 88 | + | {"a", true}, {"a/b.txt", true}, {"dir/ünï.go", true}, {"a b/c", true}, {".github/ci.yml", true}, {long, true}, | |
| 89 | + | {"/a", false}, {"a//b", false}, {"./a", false}, {"a/..", false}, {"..", false}, {".", false}, | |
| 90 | + | {"a/../b", false}, {"tab\tname", false}, {"a\nb", false}, {"a\x00b", false}, {"a\x7fb", false}, | |
| 91 | + | {"\xff", false}, {"a/\xfe/b", false}, {long + "a", false}, {"//", false}, | |
| 92 | + | } { | |
| 93 | + | _, err := domain.ParseTreePath(tt.path) | |
| 94 | + | assert.Equal(t, tt.ok, err == nil, "ParseTreePath(%q)", tt.path) | |
| 95 | + | if !tt.ok { | |
| 96 | + | assert.ErrorIs(t, err, domain.ErrInvalidPath, "%q", tt.path) | |
| 97 | + | } | |
| 98 | + | assert.Equal(t, tt.ok, domain.IsAddressablePath(tt.path), "IsAddressablePath(%q)", tt.path) | |
| 99 | + | assert.Equal(t, tt.ok, domain.IsAddressablePathBytes([]byte(tt.path)), "IsAddressablePathBytes(%q)", tt.path) | |
| 80 | 100 | } | |
| 101 | + | ||
| 102 | + | // The exceptions. | |
| 103 | + | _, err := domain.ParseTreePath("") | |
| 104 | + | assert.NoError(t, err, "the root parses") | |
| 105 | + | assert.False(t, domain.IsAddressablePath(""), "the root is not addressable") | |
| 106 | + | assert.False(t, domain.IsAddressablePathBytes(nil)) | |
| 107 | + | _, err = domain.ParseTreePath("a/") | |
| 108 | + | assert.NoError(t, err, "one trailing slash is dropped") | |
| 109 | + | assert.False(t, domain.IsAddressablePath("a/"), "an addressable path has none") | |
| 110 | + | assert.False(t, domain.IsAddressablePathBytes([]byte("a/"))) | |
| 111 | + | _, err = domain.ParseTreePath("/") | |
| 112 | + | assert.NoError(t, err, `"/" is the root with its trailing slash dropped`) | |
| 113 | + | assert.False(t, domain.IsAddressablePath("/")) | |
| 81 | 114 | } | |
| 82 | 115 | ||
| 83 | - | func TestIsAddressablePath(t *testing.T) { | |
| 84 | - | for _, ok := range []string{"a", "a/b.txt", "dir/ünï.go", "a b/c", ".github/ci.yml"} { | |
| 85 | - | assert.True(t, domain.IsAddressablePath(ok), "%q", ok) | |
| 86 | - | } | |
| 87 | - | for _, bad := range []string{"", "/", "a/", "/a", "a//b", "a/../b", "./a", "..", "tab\tname", "a\nb", "a\x00b", "a\x7fb", "\xff", "a/\xfe/b", strings.Repeat("a", 4097)} { | |
| 88 | - | assert.False(t, domain.IsAddressablePath(bad), "%q", bad) | |
| 116 | + | func TestPathListAdd(t *testing.T) { | |
| 117 | + | for _, tt := range []struct { | |
| 118 | + | name string | |
| 119 | + | adds []string | |
| 120 | + | maxPaths, maxBytes int | |
| 121 | + | want []string | |
| 122 | + | wantStopAt int // index of the Add that returned false, -1 for none | |
| 123 | + | truncated bool | |
| 124 | + | }{ | |
| 125 | + | {"all fit", []string{"a", "b/c"}, 10, 100, []string{"a", "b/c"}, -1, false}, | |
| 126 | + | {"not addressable are skipped", []string{"a", "", "b//c", "d/", "e\nf", "g"}, 10, 100, []string{"a", "g"}, -1, false}, | |
| 127 | + | {"skipped paths do not count", []string{"", "..", "a"}, 1, 1, []string{"a"}, -1, false}, | |
| 128 | + | {"exactly maxPaths", []string{"a", "b"}, 2, 100, []string{"a", "b"}, -1, false}, | |
| 129 | + | {"over maxPaths", []string{"a", "b", "c"}, 2, 100, []string{"a", "b"}, 2, true}, | |
| 130 | + | {"exactly maxBytes", []string{"ab", "cd"}, 10, 4, []string{"ab", "cd"}, -1, false}, | |
| 131 | + | {"over maxBytes", []string{"ab", "cde", "f"}, 10, 4, []string{"ab"}, 1, true}, | |
| 132 | + | {"nothing fits", []string{"abc"}, 10, 2, nil, 0, true}, | |
| 133 | + | {"no paths allowed", []string{"a"}, 0, 100, nil, 0, true}, | |
| 134 | + | {"unaddressable past the limit is no truncation", []string{"a", "b/"}, 1, 100, []string{"a"}, -1, false}, | |
| 135 | + | } { | |
| 136 | + | t.Run(tt.name, func(t *testing.T) { | |
| 137 | + | var l domain.PathList | |
| 138 | + | stop := -1 | |
| 139 | + | for i, p := range tt.adds { | |
| 140 | + | if !l.Add(p, tt.maxPaths, tt.maxBytes) { | |
| 141 | + | stop = i | |
| 142 | + | break | |
| 143 | + | } | |
| 144 | + | } | |
| 145 | + | assert.Equal(t, tt.want, l.Paths) | |
| 146 | + | assert.Equal(t, tt.wantStopAt, stop) | |
| 147 | + | assert.Equal(t, tt.truncated, l.Truncated) | |
| 148 | + | }) | |
| 89 | 149 | } | |
| 90 | - | assert.True(t, domain.IsAddressablePath(strings.Repeat("a", 4096))) | |
| 91 | 150 | } | |
| 92 | 151 | ||
| 93 | 152 | func TestEntryKindOf(t *testing.T) { | |
| @@ -73,21 +73,80 @@ func TestParseTreePath(t *testing.T) { | |||||
| 73 | require.NoError(t, err) | 73 | require.NoError(t, err) | ||
| 74 | assert.Equal(t, "a/b", p.String()) | 74 | assert.Equal(t, "a/b", p.String()) | ||
| 75 | assert.Equal(t, "a/b/c", p.Join("c")) | 75 | assert.Equal(t, "a/b/c", p.Join("c")) | ||
| 76 | + | } | |||
| 76 | 77 | ||||
| 77 | - | for _, bad := range []string{"/a", "a//b", "./a", "a/..", "..", ".", "a\x00b", "a\nb", "\xff", strings.Repeat("a", 4097), "//"} { | 78 | + | // ParseTreePath and IsAddressablePath share one rule. The only differences |
| 78 | - | _, err := domain.ParseTreePath(bad) | 79 | + | // are the root ("" is a valid path to parse, and not addressable) and the |
| 79 | - | assert.ErrorIs(t, err, domain.ErrInvalidPath, "%q", bad) | 80 | + | // trailing slash (ParseTreePath drops one, IsAddressablePath wants the path |
| 81 | + | // as it is). | |||
| 82 | + | func TestTreePathRule(t *testing.T) { | |||
| 83 | + | long := strings.Repeat("a", 4096) | |||
| 84 | + | for _, tt := range []struct { | |||
| 85 | + | path string | |||
| 86 | + | ok bool // parses, and is addressable | |||
| 87 | + | }{ | |||
| 88 | + | {"a", true}, {"a/b.txt", true}, {"dir/ünï.go", true}, {"a b/c", true}, {".github/ci.yml", true}, {long, true}, | |||
| 89 | + | {"/a", false}, {"a//b", false}, {"./a", false}, {"a/..", false}, {"..", false}, {".", false}, | |||
| 90 | + | {"a/../b", false}, {"tab\tname", false}, {"a\nb", false}, {"a\x00b", false}, {"a\x7fb", false}, | |||
| 91 | + | {"\xff", false}, {"a/\xfe/b", false}, {long + "a", false}, {"//", false}, | |||
| 92 | + | } { | |||
| 93 | + | _, err := domain.ParseTreePath(tt.path) | |||
| 94 | + | assert.Equal(t, tt.ok, err == nil, "ParseTreePath(%q)", tt.path) | |||
| 95 | + | if !tt.ok { | |||
| 96 | + | assert.ErrorIs(t, err, domain.ErrInvalidPath, "%q", tt.path) | |||
| 97 | + | } | |||
| 98 | + | assert.Equal(t, tt.ok, domain.IsAddressablePath(tt.path), "IsAddressablePath(%q)", tt.path) | |||
| 99 | + | assert.Equal(t, tt.ok, domain.IsAddressablePathBytes([]byte(tt.path)), "IsAddressablePathBytes(%q)", tt.path) | |||
| 80 | } | 100 | } | ||
| 101 | + | ||||
| 102 | + | // The exceptions. | |||
| 103 | + | _, err := domain.ParseTreePath("") | |||
| 104 | + | assert.NoError(t, err, "the root parses") | |||
| 105 | + | assert.False(t, domain.IsAddressablePath(""), "the root is not addressable") | |||
| 106 | + | assert.False(t, domain.IsAddressablePathBytes(nil)) | |||
| 107 | + | _, err = domain.ParseTreePath("a/") | |||
| 108 | + | assert.NoError(t, err, "one trailing slash is dropped") | |||
| 109 | + | assert.False(t, domain.IsAddressablePath("a/"), "an addressable path has none") | |||
| 110 | + | assert.False(t, domain.IsAddressablePathBytes([]byte("a/"))) | |||
| 111 | + | _, err = domain.ParseTreePath("/") | |||
| 112 | + | assert.NoError(t, err, `"/" is the root with its trailing slash dropped`) | |||
| 113 | + | assert.False(t, domain.IsAddressablePath("/")) | |||
| 81 | } | 114 | } | ||
| 82 | 115 | ||||
| 83 | - | func TestIsAddressablePath(t *testing.T) { | 116 | + | func TestPathListAdd(t *testing.T) { |
| 84 | - | for _, ok := range []string{"a", "a/b.txt", "dir/ünï.go", "a b/c", ".github/ci.yml"} { | 117 | + | for _, tt := range []struct { |
| 85 | - | assert.True(t, domain.IsAddressablePath(ok), "%q", ok) | 118 | + | name string |
| 86 | - | } | 119 | + | adds []string |
| 87 | - | for _, bad := range []string{"", "/", "a/", "/a", "a//b", "a/../b", "./a", "..", "tab\tname", "a\nb", "a\x00b", "a\x7fb", "\xff", "a/\xfe/b", strings.Repeat("a", 4097)} { | 120 | + | maxPaths, maxBytes int |
| 88 | - | assert.False(t, domain.IsAddressablePath(bad), "%q", bad) | 121 | + | want []string |
| 122 | + | wantStopAt int // index of the Add that returned false, -1 for none | |||
| 123 | + | truncated bool | |||
| 124 | + | }{ | |||
| 125 | + | {"all fit", []string{"a", "b/c"}, 10, 100, []string{"a", "b/c"}, -1, false}, | |||
| 126 | + | {"not addressable are skipped", []string{"a", "", "b//c", "d/", "e\nf", "g"}, 10, 100, []string{"a", "g"}, -1, false}, | |||
| 127 | + | {"skipped paths do not count", []string{"", "..", "a"}, 1, 1, []string{"a"}, -1, false}, | |||
| 128 | + | {"exactly maxPaths", []string{"a", "b"}, 2, 100, []string{"a", "b"}, -1, false}, | |||
| 129 | + | {"over maxPaths", []string{"a", "b", "c"}, 2, 100, []string{"a", "b"}, 2, true}, | |||
| 130 | + | {"exactly maxBytes", []string{"ab", "cd"}, 10, 4, []string{"ab", "cd"}, -1, false}, | |||
| 131 | + | {"over maxBytes", []string{"ab", "cde", "f"}, 10, 4, []string{"ab"}, 1, true}, | |||
| 132 | + | {"nothing fits", []string{"abc"}, 10, 2, nil, 0, true}, | |||
| 133 | + | {"no paths allowed", []string{"a"}, 0, 100, nil, 0, true}, | |||
| 134 | + | {"unaddressable past the limit is no truncation", []string{"a", "b/"}, 1, 100, []string{"a"}, -1, false}, | |||
| 135 | + | } { | |||
| 136 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 137 | + | var l domain.PathList | |||
| 138 | + | stop := -1 | |||
| 139 | + | for i, p := range tt.adds { | |||
| 140 | + | if !l.Add(p, tt.maxPaths, tt.maxBytes) { | |||
| 141 | + | stop = i | |||
| 142 | + | break | |||
| 143 | + | } | |||
| 144 | + | } | |||
| 145 | + | assert.Equal(t, tt.want, l.Paths) | |||
| 146 | + | assert.Equal(t, tt.wantStopAt, stop) | |||
| 147 | + | assert.Equal(t, tt.truncated, l.Truncated) | |||
| 148 | + | }) | |||
| 89 | } | 149 | } | ||
| 90 | - | assert.True(t, domain.IsAddressablePath(strings.Repeat("a", 4096))) | |||
| 91 | } | 150 | } | ||
| 92 | 151 | ||||
| 93 | func TestEntryKindOf(t *testing.T) { | 152 | func TestEntryKindOf(t *testing.T) { | ||