From d6dc7b7b33b7e8e9c3843cda30aa8c79d9ba7cd5 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 27 Sep 2026 06:33:02 +0100 Subject: [PATCH] Read compact npm lockfiles with a JSON fallback The v2/v3 line scanner needs npm's own formatting, and the format check matched indented text, so a compact package-lock.json parsed to zero dependencies. Keep the scanner for canonical lockfiles and decode the document when it reads nothing. --- internal/npm/npm.go | 241 +++++++++++++++++++++++++++------------ internal/npm/npm_test.go | 32 ++++++ npm_bench_test.go | 4 +- npm_test.go | 75 ++++++++++++ 4 files changed, 281 insertions(+), 71 deletions(-) diff --git a/internal/npm/npm.go b/internal/npm/npm.go index 0395fb1..234fe25 100644 --- a/internal/npm/npm.go +++ b/internal/npm/npm.go @@ -3,6 +3,7 @@ package npm import ( "bytes" "encoding/json" + "errors" "iter" "net/url" "strings" @@ -162,26 +163,32 @@ type packageLockRoot struct { PeerDependencies map[string]json.RawMessage `json:"peerDependencies"` } +var packageLockPackagesKey = []byte(`"packages"`) + +var errPackageLock = errors.New("malformed JSON") + func (p *npmPackageLockParser) Parse(filename string, content []byte) (*core.Result, error) { - // Quick check for lockfile version to determine parsing strategy - // v3 (lockfileVersion >= 2 with packages) uses line-based parsing - // v1 uses JSON parsing for nested dependencies - const headerPeekSize = 200 - const packagesPeekSize = 600 - header := string(content[:min(headerPeekSize, len(content))]) - - // v2+ with packages section uses line-based v3 parsing - if strings.Contains(header, `"lockfileVersion": 3`) || - (strings.Contains(header, `"lockfileVersion": 2`) && strings.Contains(string(content[:min(packagesPeekSize, len(content))]), `"packages"`)) { - return &core.Result{Dependencies: parsePackageLockV3Lines(content)}, nil - } - - // v1 format uses JSON (nested dependencies make line parsing complex) - var lock packageLockJSON - if err := json.Unmarshal(content, &lock); err != nil { + // Without a packages section the lockfile is v1: nested dependencies only. + if !bytes.Contains(content, packageLockPackagesKey) { + var lock packageLockJSON + if err := json.Unmarshal(content, &lock); err != nil { + return nil, &core.ParseError{Filename: filename, Err: err} + } + return &core.Result{Dependencies: parsePackageLockV1(lock.Dependencies)}, nil + } + + // npm writes one key per line, which the line scanner reads without + // decoding the document. + if deps := parsePackageLockV3Lines(content); len(deps) > 0 { + return &core.Result{Dependencies: deps}, nil + } + + // Any other formatting, compact JSON included. + deps, err := decodePackageLock(content) + if err != nil { return nil, &core.ParseError{Filename: filename, Err: err} } - return &core.Result{Dependencies: parsePackageLockV1(lock.Dependencies)}, nil + return &core.Result{Dependencies: deps}, nil } func parsePackageLockV1(deps map[string]packageLockDep) []core.Dependency { @@ -225,54 +232,48 @@ func appendPackageLockV1(result []core.Dependency, deps map[string]packageLockDe return result } -// v3PackageEntry holds the state accumulated while parsing a single package -// entry in the v3 lockfile format. -type v3PackageEntry struct { - path string - version string - integrity string - resolved string - dev bool - optional bool - devOptional bool - link bool +// packageLockEntry is one entry of the v2/v3 "packages" object, filled either +// by the line scanner or by the JSON decoder. Path holds the install path the +// entry is keyed by. +type packageLockEntry struct { + Path string `json:"-"` + Version string `json:"version"` + Integrity string `json:"integrity"` + Resolved string `json:"resolved"` + Dev bool `json:"dev"` + Optional bool `json:"optional"` + DevOptional bool `json:"devOptional"` + Link bool `json:"link"` } -func (e *v3PackageEntry) reset(path string) { - e.path = path - e.version = "" - e.integrity = "" - e.resolved = "" - e.dev = false - e.optional = false - e.devOptional = false - e.link = false +func (e *packageLockEntry) reset(path string) { + *e = packageLockEntry{Path: path} } -func (e *v3PackageEntry) hasContent() bool { - return e.path != "" && (e.version != "" || e.link) +func (e *packageLockEntry) hasContent() bool { + return e.Path != "" && (e.Version != "" || e.Link) } -func (e *v3PackageEntry) toDependency(directDependencies map[string]bool) (core.Dependency, bool) { - name := extractPackageName(e.path) - if name == "" { +func (e *packageLockEntry) toDependency(directDependencies map[string]bool) (core.Dependency, bool) { + name := extractPackageName(e.Path) + if name == "" || (e.Version == "" && !e.Link) { return core.Dependency{}, false } scope := core.Runtime - if e.dev || e.devOptional { + if e.Dev || e.DevOptional { scope = core.Development - } else if e.optional { + } else if e.Optional { scope = core.Optional } - topLevel := !strings.Contains(strings.TrimPrefix(e.path, "node_modules/"), "node_modules/") + topLevel := !strings.Contains(strings.TrimPrefix(e.Path, "node_modules/"), "node_modules/") direct := topLevel && directDependencies[name] return core.Dependency{ Name: name, - Version: e.version, + Version: e.Version, Scope: scope, - Integrity: e.integrity, + Integrity: e.Integrity, Direct: direct, - RegistryURL: e.resolved, + RegistryURL: e.Resolved, }, true } @@ -290,7 +291,7 @@ func parsePackageLockDirectDependencies(content []byte) map[string]bool { if key == "packages" { return decodePackageLockRootDependencies(decoder) } - if !discardJSONValue(decoder) { + if !skipJSONValue(decoder) { return nil } } @@ -309,9 +310,13 @@ func decodePackageLockRootDependencies(decoder *json.Decoder) map[string]bool { return nil } if path == "" { - return decodePackageLockRoot(decoder) + declared, err := decodePackageLockRoot(decoder) + if err != nil { + return nil + } + return declared } - if !discardJSONValue(decoder) { + if !skipJSONValue(decoder) { return nil } } @@ -319,10 +324,10 @@ func decodePackageLockRootDependencies(decoder *json.Decoder) map[string]bool { return nil } -func decodePackageLockRoot(decoder *json.Decoder) map[string]bool { +func decodePackageLockRoot(decoder *json.Decoder) (map[string]bool, error) { var root packageLockRoot if err := decoder.Decode(&root); err != nil { - return nil + return nil, err } directDependencies := make(map[string]bool) @@ -330,7 +335,7 @@ func decodePackageLockRoot(decoder *json.Decoder) map[string]bool { collectPackageLockDependencyNames(directDependencies, root.DevDependencies) collectPackageLockDependencyNames(directDependencies, root.OptionalDependencies) collectPackageLockDependencyNames(directDependencies, root.PeerDependencies) - return directDependencies + return directDependencies, nil } func decodeJSONObjectOpening(decoder *json.Decoder) bool { @@ -347,11 +352,6 @@ func decodeJSONKey(decoder *json.Decoder) (string, bool) { return key, ok } -func discardJSONValue(decoder *json.Decoder) bool { - var discarded json.RawMessage - return decoder.Decode(&discarded) == nil -} - func collectPackageLockDependencyNames(directDependencies map[string]bool, dependencies map[string]json.RawMessage) { for name := range dependencies { directDependencies[name] = true @@ -360,28 +360,28 @@ func collectPackageLockDependencyNames(directDependencies map[string]bool, depen // updateFromLine reads a trimmed line and updates the entry's fields. // Returns true if the line was consumed. -func (e *v3PackageEntry) updateFromLine(trimmed string) bool { +func (e *packageLockEntry) updateFromLine(trimmed string) bool { switch { case strings.HasPrefix(trimmed, `"version"`): if v := extractJSONStringValue(trimmed); v != "" { - e.version = v + e.Version = v } case strings.HasPrefix(trimmed, `"integrity"`): if v := extractJSONStringValue(trimmed); v != "" { - e.integrity = v + e.Integrity = v } case strings.HasPrefix(trimmed, `"resolved"`): if v := extractJSONStringValue(trimmed); v != "" { - e.resolved = v + e.Resolved = v } case strings.HasPrefix(trimmed, `"dev": true`): - e.dev = true + e.Dev = true case strings.HasPrefix(trimmed, `"optional": true`): - e.optional = true + e.Optional = true case strings.HasPrefix(trimmed, `"devOptional": true`): - e.devOptional = true + e.DevOptional = true case strings.HasPrefix(trimmed, `"link": true`): - e.link = true + e.Link = true default: return false } @@ -436,10 +436,10 @@ func parsePackageLockV3Lines(content []byte) []core.Dependency { return deps } -func packageLockV3Entries(content string) iter.Seq[v3PackageEntry] { - return func(yield func(v3PackageEntry) bool) { +func packageLockV3Entries(content string) iter.Seq[packageLockEntry] { + return func(yield func(packageLockEntry) bool) { inPackages := false - var entry v3PackageEntry + var entry packageLockEntry for line := range strings.SplitSeq(content, "\n") { trimmed := strings.TrimSpace(line) @@ -493,6 +493,107 @@ func extractJSONStringValue(line string) string { return rest[start+1 : start+1+end] } +// decodePackageLock decodes the document, for lockfiles the line scanner cannot +// read. v2 and v3 list every installed package under "packages"; v1 only has +// the nested "dependencies" tree. +func decodePackageLock(content []byte) ([]core.Dependency, error) { + decoder := json.NewDecoder(bytes.NewReader(content)) + if !decodeJSONObjectOpening(decoder) { + return nil, errPackageLock + } + + var tree map[string]packageLockDep + for decoder.More() { + key, ok := decodeJSONKey(decoder) + if !ok { + return nil, errPackageLock + } + switch key { + case "packages": + if !decodeJSONObjectOpening(decoder) { + return nil, errPackageLock + } + return decodePackageLockEntries(decoder) + case "dependencies": + if err := decoder.Decode(&tree); err != nil { + return nil, err + } + default: + if !skipJSONValue(decoder) { + return nil, errPackageLock + } + } + } + + return parsePackageLockV1(tree), nil +} + +// decodePackageLockEntries reads the entries of an open "packages" object in +// document order. The root entry is converted last because it can appear after +// the packages it declares. +func decodePackageLockEntries(decoder *json.Decoder) ([]core.Dependency, error) { + var entries []packageLockEntry + var directDependencies map[string]bool + + for decoder.More() { + path, ok := decodeJSONKey(decoder) + if !ok { + return nil, errPackageLock + } + if path == "" { + declared, err := decodePackageLockRoot(decoder) + if err != nil { + return nil, err + } + directDependencies = declared + continue + } + entries = append(entries, packageLockEntry{Path: path}) + if err := decoder.Decode(&entries[len(entries)-1]); err != nil { + return nil, err + } + } + + deps := make([]core.Dependency, 0, len(entries)) + for i := range entries { + if dep, ok := entries[i].toDependency(directDependencies); ok { + deps = append(deps, dep) + } + } + if len(deps) == 0 { + return nil, nil + } + return deps, nil +} + +// skipJSONValue reads past the next value without copying it. +func skipJSONValue(decoder *json.Decoder) bool { + token, err := decoder.Token() + if err != nil { + return false + } + depth := 0 + switch token { + case json.Delim('{'), json.Delim('['): + depth = 1 + default: + return true + } + for depth > 0 { + token, err = decoder.Token() + if err != nil { + return false + } + switch token { + case json.Delim('{'), json.Delim('['): + depth++ + case json.Delim('}'), json.Delim(']'): + depth-- + } + } + return true +} + // extractPackageName extracts the package name from a node_modules path. func extractPackageName(path string) string { // Remove leading node_modules/ diff --git a/internal/npm/npm_test.go b/internal/npm/npm_test.go index b2a99b6..ad8250a 100644 --- a/internal/npm/npm_test.go +++ b/internal/npm/npm_test.go @@ -2,6 +2,7 @@ package npm import ( "os" + "reflect" "testing" "github.com/git-pkgs/manifests/internal/core" @@ -1207,3 +1208,34 @@ func TestYarnV4Lock(t *testing.T) { t.Error("workspace package should be excluded") } } + +// TestPackageLockPathsAgree keeps the line scanner and the JSON decoder from +// drifting apart on the lockfiles npm itself writes. +func TestPackageLockPathsAgree(t *testing.T) { + paths := []string{ + "../../testdata/npm/npm-lockfile-version-1/package-lock.json", + "../../testdata/npm/npm-lockfile-version-2/package-lock.json", + "../../testdata/npm/npm-lockfile-version-3/package-lock.json", + "../../testdata/npm/npm-local-file/package-lock.json", + "../../testdata/misc/multiple_versions/package-lock.json", + } + for _, path := range paths { + t.Run(path, func(t *testing.T) { + content, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + scanned := parsePackageLockV3Lines(content) + if len(scanned) == 0 { + t.Fatal("line scanner read no dependencies") + } + decoded, err := decodePackageLock(content) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(scanned, decoded) { + t.Fatalf("line scanner gave %+v, decoder gave %+v", scanned, decoded) + } + }) + } +} diff --git a/npm_bench_test.go b/npm_bench_test.go index 8490869..d9e2cb2 100644 --- a/npm_bench_test.go +++ b/npm_bench_test.go @@ -41,7 +41,8 @@ func TestNPMLargeLockfile(t *testing.T) { const count = 10000 for _, format := range []int{1, 3} { t.Run(fmt.Sprintf("v%d", format), func(t *testing.T) { - result, err := Parse("package-lock.json", npmLockHistory(t, format, count)) + data := npmLockHistory(t, format, count) + result, err := Parse("package-lock.json", data) if err != nil { t.Fatal(err) } @@ -61,6 +62,7 @@ func TestNPMLargeLockfile(t *testing.T) { t.Fatalf("unexpected direct flag: %+v", dep) } } + requireCompactEquivalence(t, "package-lock.json", data, result.Dependencies) }) } } diff --git a/npm_test.go b/npm_test.go index afbcba9..45a6ccd 100644 --- a/npm_test.go +++ b/npm_test.go @@ -2,6 +2,7 @@ package manifests import ( "bytes" + "encoding/json" "fmt" "os" "reflect" @@ -213,3 +214,77 @@ func TestNPMV2LockfileCRLF(t *testing.T) { }) } } + +func sortedDependencies(deps []Dependency) []Dependency { + sorted := append([]Dependency(nil), deps...) + sort.Slice(sorted, func(i, j int) bool { + a, b := sorted[i], sorted[j] + if a.Name != b.Name { + return a.Name < b.Name + } + return a.Version < b.Version + }) + return sorted +} + +// requireCompactEquivalence reparses the lockfile with its whitespace removed +// and requires the same dependencies. +func requireCompactEquivalence(t *testing.T, filename string, content []byte, want []Dependency) { + t.Helper() + var compact bytes.Buffer + if err := json.Compact(&compact, content); err != nil { + t.Fatal(err) + } + result, err := Parse(filename, compact.Bytes()) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(sortedDependencies(result.Dependencies), sortedDependencies(want)) { + t.Fatalf("compact JSON gave %+v, want %+v", result.Dependencies, want) + } +} + +func TestNPMLockfileCompactJSON(t *testing.T) { + paths := []string{ + "npm/package-lock.json", + "npm/npm-lockfile-version-1/package-lock.json", + "npm/npm-lockfile-version-2/package-lock.json", + "npm/npm-lockfile-version-3/package-lock.json", + "npm/npm-local-file/package-lock.json", + "misc/multiple_versions/package-lock.json", + } + for _, path := range paths { + t.Run(path, func(t *testing.T) { + content, err := os.ReadFile("testdata/" + path) + if err != nil { + t.Fatal(err) + } + want, err := Parse("package-lock.json", content) + if err != nil { + t.Fatal(err) + } + if len(want.Dependencies) == 0 { + t.Fatal("fixture has no dependencies") + } + requireCompactEquivalence(t, "package-lock.json", content, want.Dependencies) + }) + } +} + +func TestNPMLockfileRootEntryLast(t *testing.T) { + content := []byte(`{"lockfileVersion":3,"packages":{` + + `"node_modules/direct":{"version":"1.0.0"},` + + `"node_modules/transitive":{"version":"2.0.0"},` + + `"":{"name":"example","dependencies":{"direct":"^1.0.0"}}}}`) + result, err := Parse("package-lock.json", content) + if err != nil { + t.Fatal(err) + } + want := []Dependency{ + {Name: "direct", Version: "1.0.0", Scope: Runtime, Direct: true, PURL: "pkg:npm/direct@1.0.0"}, + {Name: "transitive", Version: "2.0.0", Scope: Runtime, PURL: "pkg:npm/transitive@2.0.0"}, + } + if !reflect.DeepEqual(result.Dependencies, want) { + t.Fatalf("got %+v, want %+v", result.Dependencies, want) + } +}