Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

codeintel: fix panic in extracting JARs with absolute paths #24652

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

Strum355
Copy link
Contributor

@Strum355 Strum355 commented Sep 6, 2021

@Strum355 Strum355 added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) dependency-indexing labels Sep 6, 2021
@Strum355 Strum355 requested a review from olafurpg September 6, 2021 15:39
@Strum355 Strum355 self-assigned this Sep 6, 2021
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Sep 6, 2021

Notifying subscribers in CODENOTIFY files for diff bf7289e...e8a6f7e.

Notify File(s)
@indradhanush cmd/gitserver/server/vcs_syncer_jvm_packages.go
cmd/gitserver/server/vcs_syncer_jvm_packages_test.go
@unknwon cmd/gitserver/server/vcs_syncer_jvm_packages.go
cmd/gitserver/server/vcs_syncer_jvm_packages_test.go

Copy link
Member

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Great job investigating the root cause of this issue @Strum355!

@@ -368,14 +357,19 @@ func unzipJarFile(jarPath, destination string) (err error) {
// `file.Name` docstring.
continue
}
outputPath = path.Join(destination, file.Name)
if strings.HasPrefix(file.Name, "/") {
// Skip absolute paths. While they are extracted relative to `destination`,
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth including a link to the upstream Go issue for more context.

@Strum355 Strum355 force-pushed the nsc/jar-extract-panic branch from dd59b81 to e8a6f7e Compare September 8, 2021 10:51
@Strum355 Strum355 enabled auto-merge (squash) September 8, 2021 10:53
@Strum355 Strum355 merged commit ddd06c5 into main Sep 8, 2021
@Strum355 Strum355 deleted the nsc/jar-extract-panic branch September 8, 2021 10:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependency-indexing team/graph Graph Team (previously Code Intel/Language Tools/Language Platform)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants