Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions external-providers/dotnet-external-provider/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mO
github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/iancoleman/orderedmap v0.3.0 h1:5cbR2grmZR/DiVt+VJopEhtVs9YGInGIxAoMJn+Ichc=
github.com/iancoleman/orderedmap v0.3.0/go.mod h1:XuLcCUkdL5owUCQeF2Ue9uuw1EptkJDkXXS7VoV7XGE=
github.com/konveyor/analyzer-lsp v0.7.0-alpha.2.0.20250625194402-05dca9b4ac43/go.mod h1:/7nwwqN27iODJy/PBai9W16KH91LrPGx1nwu21+rCOg=
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
javaDepSourceInternal = "internal"
javaDepSourceOpenSource = "open-source"
providerSpecificConfigOpenSourceDepListKey = "depOpenSourceLabelsFile"
providerSpecificConfigMavenIndexPath = "mavenIndexPath"
providerSpecificConfigExcludePackagesKey = "excludePackages"
)

Expand Down Expand Up @@ -661,26 +662,26 @@ func extractSubmoduleTrees(lines []string) [][]string {
func (p *javaServiceClient) discoverDepsFromJars(path string, ll map[uri.URI][]konveyor.DepDAGItem, disableMavenSearch bool) {
// for binaries we only find JARs embedded in archive
w := walker{
deps: ll,
depToLabels: p.depToLabels,
m2RepoPath: p.mvnLocalRepo,
seen: map[string]bool{},
initialPath: path,
log: p.log,
disableMavenSearch: disableMavenSearch,
deps: ll,
depToLabels: p.depToLabels,
m2RepoPath: p.mvnLocalRepo,
seen: map[string]bool{},
initialPath: path,
log: p.log,
mvnIndexPath: p.mvnIndexPath,
}
filepath.WalkDir(path, w.walkDirForJar)
}

type walker struct {
deps map[uri.URI][]provider.DepDAGItem
depToLabels map[string]*depLabelItem
m2RepoPath string
initialPath string
seen map[string]bool
pomPaths []string
log logr.Logger
disableMavenSearch bool
deps map[uri.URI][]provider.DepDAGItem
depToLabels map[string]*depLabelItem
m2RepoPath string
initialPath string
seen map[string]bool
pomPaths []string
log logr.Logger
mvnIndexPath string
}

func (w *walker) walkDirForJar(path string, info fs.DirEntry, err error) error {
Expand All @@ -699,7 +700,7 @@ func (w *walker) walkDirForJar(path string, info fs.DirEntry, err error) error {
d := provider.Dep{
Name: info.Name(),
}
artifact, _ := toDependency(context.TODO(), w.log, w.depToLabels, path, w.disableMavenSearch)
artifact, _ := toDependency(context.TODO(), w.log, path, w.mvnIndexPath)
if (artifact != javaArtifact{}) {
d.Name = fmt.Sprintf("%s.%s", artifact.GroupId, artifact.ArtifactId)
d.Version = artifact.Version
Expand Down Expand Up @@ -755,13 +756,14 @@ func (w *walker) walkDirForJar(path string, info fs.DirEntry, err error) error {

func (p *javaServiceClient) discoverPoms(pathStart string, ll map[uri.URI][]konveyor.DepDAGItem) []string {
w := walker{
deps: ll,
depToLabels: p.depToLabels,
m2RepoPath: "",
seen: map[string]bool{},
initialPath: pathStart,
pomPaths: []string{},
log: p.log,
deps: ll,
depToLabels: p.depToLabels,
m2RepoPath: "",
seen: map[string]bool{},
initialPath: pathStart,
pomPaths: []string{},
log: p.log,
mvnIndexPath: p.mvnIndexPath,
}
filepath.WalkDir(pathStart, w.walkDirForPom)
return w.pomPaths
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ func (p *javaProvider) Init(ctx context.Context, log logr.Logger, config provide
}

disableMavenSearch, ok := config.ProviderSpecificConfig[DISABLE_MAVEN_SEARCH].(bool)
mavenIndexPath, ok := config.ProviderSpecificConfig[providerSpecificConfigMavenIndexPath].(string)
Comment on lines 288 to +289
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify handling of missing or empty mavenIndexPath.

The ok variable is captured but not checked. If the config key is missing or the value is not a string, mavenIndexPath will be an empty string. Given that this PR replaces Maven Central HTTP search with index-based lookup, an empty or missing mavenIndexPath could cause silent failures in downstream decompilation and dependency resolution flows.

Consider adding validation and logging:

 disableMavenSearch, ok := config.ProviderSpecificConfig[DISABLE_MAVEN_SEARCH].(bool)
-mavenIndexPath, ok := config.ProviderSpecificConfig[providerSpecificConfigMavenIndexPath].(string)
+mavenIndexPath, ok := config.ProviderSpecificConfig[providerSpecificConfigMavenIndexPath].(string)
+if !ok || mavenIndexPath == "" {
+	log.V(5).Info("mavenIndexPath not configured, index-based dependency resolution will be unavailable")
+}

Run this script to verify how downstream code handles an empty mavenIndexPath:


🏁 Script executed:

#!/bin/bash
# Description: Check how decompileJava and decompile functions handle empty mavenIndexPath

# Search for decompileJava function definition and usage
ast-grep --pattern $'func decompileJava($$$, mavenIndexPath string) $$$'

# Search for decompile function definition and usage
ast-grep --pattern $'func decompile($$$, mavenIndexPath string) $$$'

# Check if there are any validation checks for empty mavenIndexPath
rg -nP --type=go 'mavenIndexPath\s*(==|!=)\s*""' -A3 -B3

Length of output: 0



---

🏁 Script executed:

```shell
#!/bin/bash
# Find all function definitions containing "decompile"
rg -n "^func.*decompile" --type=go

# Find where mavenIndexPath is used
rg -n "mavenIndexPath" --type=go -A2 -B2

Length of output: 11596



---

🏁 Script executed:

```shell
#!/bin/bash
# Find all calls to decompileJava to see the pattern for cleanBin parameter
rg -n "decompileJava\(" --type=go -B2 -A2

# Check what cleanBin parameter should be by looking at its usage
rg -n "cleanBin" --type=go -B3 -A3

Length of output: 5021


Fix critical bug: Line 369 passes ok instead of cleanBin to decompileJava.

At line 366, cleanBin and ok are assigned from the type assertion: cleanBin, ok := config.ProviderSpecificConfig[CLEAN_EXPLODED_BIN_OPTION].(bool). The variable cleanBin holds the actual configuration value, while ok indicates whether the type assertion succeeded.

Line 369 incorrectly passes ok as the cleanBin parameter:

depLocation, sourceLocation, err := decompileJava(ctx, log, fernflower,
    config.Location, getMavenLocalRepoPath(mavenSettingsFile), ok, mavenIndexPath)

This should pass cleanBin instead. Line 379 confirms they are distinct: if ok && cleanBin { shows both variables are checked separately. The decompileJava function uses this parameter to determine project path naming at line 177.

Regarding the original mavenIndexPath concern: defensive validation exists at service_client.go:139-141 (if p.mvnIndexPath != ""), but explicit validation and logging at config read time (line 289) would provide better clarity about missing configuration.

🤖 Prompt for AI Agents
external-providers/java-external-provider/pkg/java_external_provider/provider.go
around lines 288-289: the code performs a type assertion assigning cleanBin, ok
:= config.ProviderSpecificConfig[CLEAN_EXPLODED_BIN_OPTION].(bool) but then
mistakenly passes ok to decompileJava instead of the actual cleanBin value;
change the decompileJava call to pass cleanBin as the boolean argument (not ok).
Also add a defensive check/log when reading mavenIndexPath to validate it's
non-empty when required (log a warning or error if missing) so configuration
issues are surfaced early.


isBinary := false
var returnErr error
Expand Down Expand Up @@ -365,7 +366,7 @@ func (p *javaProvider) Init(ctx context.Context, log logr.Logger, config provide
cleanBin, ok := config.ProviderSpecificConfig[CLEAN_EXPLODED_BIN_OPTION].(bool)

depLocation, sourceLocation, err := decompileJava(ctx, log, fernflower,
config.Location, getMavenLocalRepoPath(mavenSettingsFile), ok, openSourceDepLabels, disableMavenSearch)
config.Location, getMavenLocalRepoPath(mavenSettingsFile), ok, mavenIndexPath)
if err != nil {
cancelFunc()
return nil, additionalBuiltinConfig, err
Expand Down Expand Up @@ -506,13 +507,6 @@ func (p *javaProvider) Init(ctx context.Context, log logr.Logger, config provide

m2Repo := getMavenLocalRepoPath(mavenSettingsFile)

mavenIndexPath := ""
if val, ok := config.ProviderSpecificConfig[providerSpecificConfigOpenSourceDepListKey]; ok {
if strVal, ok := val.(string); ok {
mavenIndexPath = strVal
}
}

svcClient := javaServiceClient{
rpc: rpc,
cancelFunc: cancelFunc,
Expand All @@ -539,7 +533,7 @@ func (p *javaProvider) Init(ctx context.Context, log logr.Logger, config provide
// we need to do this for jdtls to correctly recognize source attachment for dep
switch svcClient.GetBuildTool() {
case maven:
err := svcClient.resolveSourcesJarsForMaven(ctx, fernflower, disableMavenSearch)
err := svcClient.resolveSourcesJarsForMaven(ctx, fernflower, mavenIndexPath)
if err != nil {
// TODO (pgaikwad): should we ignore this failure?
log.Error(err, "failed to resolve maven sources jar for location", "location", config.Location)
Expand All @@ -549,7 +543,7 @@ func (p *javaProvider) Init(ctx context.Context, log logr.Logger, config provide
if !ok {
gradleTaskFile = ""
}
err = svcClient.resolveSourcesJarsForGradle(ctx, fernflower, disableMavenSearch, gradleTaskFile.(string))
err = svcClient.resolveSourcesJarsForGradle(ctx, fernflower, disableMavenSearch, gradleTaskFile.(string), mavenIndexPath)
if err != nil {
log.Error(err, "failed to resolve gradle sources jar for location", "location", config.Location)
}
Expand Down Expand Up @@ -593,7 +587,7 @@ func (p *javaProvider) Init(ctx context.Context, log logr.Logger, config provide
return &svcClient, additionalBuiltinConfig, returnErr
}

func (s *javaServiceClient) resolveSourcesJarsForGradle(ctx context.Context, fernflower string, disableMavenSearch bool, taskFile string) error {
func (s *javaServiceClient) resolveSourcesJarsForGradle(ctx context.Context, fernflower string, disableMavenSearch bool, taskFile string, mavenIndexPath string) error {
ctx, span := tracing.StartNewSpan(ctx, "resolve-sources")
defer span.End()

Expand Down Expand Up @@ -702,7 +696,7 @@ func (s *javaServiceClient) resolveSourcesJarsForGradle(ctx context.Context, fer
outputPath: filepath.Join(filepath.Dir(artifactPath), "decompiled", jarName),
})
}
err = decompile(ctx, s.log, alwaysDecompileFilter(true), 10, decompileJobs, fernflower, "", s.depToLabels, disableMavenSearch)
err = decompile(ctx, s.log, alwaysDecompileFilter(true), 10, decompileJobs, fernflower, "", mavenIndexPath)
if err != nil {
return err
}
Expand Down Expand Up @@ -849,7 +843,7 @@ func (j *javaProvider) GetLocation(ctx context.Context, dep konveyor.Dep, file s

// resolveSourcesJarsForMaven for a given source code location, runs maven to find
// deps that don't have sources attached and decompiles them
func (s *javaServiceClient) resolveSourcesJarsForMaven(ctx context.Context, fernflower string, disableMavenSearch bool) error {
func (s *javaServiceClient) resolveSourcesJarsForMaven(ctx context.Context, fernflower string, mavenIndexPath string) error {
// TODO (pgaikwad): when we move to external provider, inherit context from parent
ctx, span := tracing.StartNewSpan(ctx, "resolve-sources")
defer span.End()
Expand Down Expand Up @@ -902,7 +896,7 @@ func (s *javaServiceClient) resolveSourcesJarsForMaven(ctx context.Context, fern
s.mvnLocalRepo, groupDirs, artifactDirs, artifact.Version, "decompiled", jarName),
})
}
err = decompile(ctx, s.log, alwaysDecompileFilter(true), 10, decompileJobs, fernflower, "", s.depToLabels, disableMavenSearch)
err = decompile(ctx, s.log, alwaysDecompileFilter(true), 10, decompileJobs, fernflower, "", mavenIndexPath)
if err != nil {
return err
}
Expand Down
Loading
Loading