Skip to content

Commit 5a19c65

Browse files
authored
Fixed a panic caused by nil dependency specs in Ruby Gemfile.lock resolver (#207)
- now treats missing specs as unresolved licenses - adds them to the invalid list - License fetch logic handles empty gem versions gracefully - tests updated - handle missing specs gracefully with mocked HTTP responses - imports and packages were refactored to enable mocking and improve test structure
1 parent 0d9c4df commit 5a19c65

File tree

2 files changed

+88
-9
lines changed

2 files changed

+88
-9
lines changed

pkg/deps/ruby.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,11 @@ func (r *GemfileLockResolver) Resolve(lockfile string, config *ConfigDeps, repor
9292

9393
// Resolve licenses for included gems
9494
for name := range include {
95-
version := specs[name].Version
95+
// Some roots may not exist in the specs graph (e.g., git-sourced gems)
96+
var version string
97+
if spec, ok := specs[name]; ok && spec != nil {
98+
version = spec.Version
99+
}
96100
if exclude, _ := config.IsExcluded(name, version); exclude {
97101
continue
98102
}
@@ -103,6 +107,7 @@ func (r *GemfileLockResolver) Resolve(lockfile string, config *ConfigDeps, repor
103107

104108
licenseID, err := fetchRubyGemsLicense(name, version)
105109
if err != nil || licenseID == "" {
110+
// Gracefully treat as unresolved license and record in report
106111
report.Skip(&Result{Dependency: name, LicenseSpdxID: Unknown, Version: version})
107112
continue
108113
}
@@ -269,6 +274,11 @@ type rubyGemsVersionInfo struct {
269274
}
270275

271276
func fetchRubyGemsLicense(name, version string) (string, error) {
277+
// If version is unknown (e.g., git-sourced), query latest gem info endpoint
278+
if strings.TrimSpace(version) == "" {
279+
url := fmt.Sprintf("https://rubygems.org/api/v1/gems/%s.json", name)
280+
return fetchRubyGemsLicenseFrom(url)
281+
}
272282
// Prefer version-specific API
273283
url := fmt.Sprintf("https://rubygems.org/api/v2/rubygems/%s/versions/%s.json", name, version)
274284
licenseID, err := fetchRubyGemsLicenseFrom(url)

pkg/deps/ruby_test.go

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,18 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
package deps_test
18+
package deps
1919

2020
import (
2121
"bufio"
2222
"embed"
23+
"io"
2324
"io/fs"
25+
"net/http"
2426
"os"
2527
"path/filepath"
2628
"strings"
2729
"testing"
28-
29-
"github.com/apache/skywalking-eyes/pkg/deps"
3030
)
3131

3232
func writeFileRuby(fileName, content string) error {
@@ -73,7 +73,7 @@ func copyRuby(assetDir, destination string) error {
7373
}
7474

7575
func TestRubyGemfileLockResolver(t *testing.T) {
76-
resolver := new(deps.GemfileLockResolver)
76+
resolver := new(GemfileLockResolver)
7777

7878
// App case: include all specs (3)
7979
{
@@ -85,12 +85,12 @@ func TestRubyGemfileLockResolver(t *testing.T) {
8585
if !resolver.CanResolve(lock) {
8686
t.Fatalf("GemfileLockResolver cannot resolve %s", lock)
8787
}
88-
cfg := &deps.ConfigDeps{Files: []string{lock}, Licenses: []*deps.ConfigDepLicense{
88+
cfg := &ConfigDeps{Files: []string{lock}, Licenses: []*ConfigDepLicense{
8989
{Name: "rake", Version: "13.0.6", License: "MIT"},
9090
{Name: "rspec", Version: "3.10.0", License: "MIT"},
9191
{Name: "rspec-core", Version: "3.10.1", License: "MIT"},
9292
}}
93-
report := deps.Report{}
93+
report := Report{}
9494
if err := resolver.Resolve(lock, cfg, &report); err != nil {
9595
t.Fatal(err)
9696
}
@@ -106,10 +106,10 @@ func TestRubyGemfileLockResolver(t *testing.T) {
106106
t.Fatal(err)
107107
}
108108
lock := filepath.Join(tmp, "Gemfile.lock")
109-
cfg := &deps.ConfigDeps{Files: []string{lock}, Licenses: []*deps.ConfigDepLicense{
109+
cfg := &ConfigDeps{Files: []string{lock}, Licenses: []*ConfigDepLicense{
110110
{Name: "rake", Version: "13.0.6", License: "MIT"},
111111
}}
112-
report := deps.Report{}
112+
report := Report{}
113113
if err := resolver.Resolve(lock, cfg, &report); err != nil {
114114
t.Fatal(err)
115115
}
@@ -118,3 +118,72 @@ func TestRubyGemfileLockResolver(t *testing.T) {
118118
}
119119
}
120120
}
121+
122+
// mock RoundTripper to control HTTP responses
123+
type roundTripFunc func(*http.Request) (*http.Response, error)
124+
125+
func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) }
126+
127+
func TestRubyMissingSpecIsSkippedGracefully(t *testing.T) {
128+
// Mock HTTP client to avoid real network: always return 404 Not Found
129+
saved := httpClientRuby
130+
httpClientRuby = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) {
131+
return &http.Response{
132+
StatusCode: http.StatusNotFound,
133+
Status: "404 Not Found",
134+
Body: io.NopCloser(strings.NewReader("{}")),
135+
Header: make(http.Header),
136+
}, nil
137+
})}
138+
defer func() { httpClientRuby = saved }()
139+
140+
// Create a Gemfile.lock where a dependency is not present in specs
141+
content := "" +
142+
"GEM\n" +
143+
" remote: https://rubygems.org/\n" +
144+
" specs:\n" +
145+
" rake (13.0.6)\n" +
146+
"\n" +
147+
"PLATFORMS\n" +
148+
" ruby\n" +
149+
"\n" +
150+
"DEPENDENCIES\n" +
151+
" rake\n" +
152+
" missing_gem\n" +
153+
"\n" +
154+
"BUNDLED WITH\n" +
155+
" 2.4.10\n"
156+
157+
dir := t.TempDir()
158+
lock := filepath.Join(dir, "Gemfile.lock")
159+
if err := writeFileRuby(lock, content); err != nil {
160+
t.Fatal(err)
161+
}
162+
163+
resolver := new(GemfileLockResolver)
164+
cfg := &ConfigDeps{Files: []string{lock}, Licenses: []*ConfigDepLicense{
165+
{Name: "rake", Version: "13.0.6", License: "MIT"}, // only rake is configured; missing_gem should be skipped
166+
}}
167+
report := Report{}
168+
if err := resolver.Resolve(lock, cfg, &report); err != nil {
169+
t.Fatal(err)
170+
}
171+
172+
if got := len(report.Resolved) + len(report.Skipped); got != 2 {
173+
t.Fatalf("expected 2 dependencies total, got %d", got)
174+
}
175+
176+
// Ensure 'missing_gem' is in skipped with empty version
177+
found := false
178+
for _, s := range report.Skipped {
179+
if s.Dependency == "missing_gem" {
180+
found = true
181+
if s.Version != "" {
182+
t.Fatalf("expected empty version for missing_gem, got %q", s.Version)
183+
}
184+
}
185+
}
186+
if !found {
187+
t.Fatalf("expected missing_gem to be marked as skipped")
188+
}
189+
}

0 commit comments

Comments
 (0)