-
Notifications
You must be signed in to change notification settings - Fork 415
Fix SCL-24273: JAR class resolution with special characters in filenames #704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
106 changes: 106 additions & 0 deletions
106
scala/compiler-shared/test/org/jetbrains/jps/incremental/scala/JarUrlEncodingTest.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
package org.jetbrains.jps.incremental.scala | ||
|
||
import org.junit.Assert._ | ||
import org.junit.Test | ||
|
||
import java.io.File | ||
import java.nio.file.{Files, Path} | ||
import java.util.Properties | ||
import scala.util.Using | ||
|
||
/** | ||
* Tests for the fixed readProperty method that handles JAR files with special characters. | ||
* | ||
* This addresses SCL-24273: Classes decompiled from JAR files with special characters | ||
* in their names are not resolved. | ||
*/ | ||
class JarUrlEncodingTest { | ||
|
||
@Test | ||
def testReadPropertyWithSpecialCharacters(): Unit = { | ||
val testCases = Seq( | ||
"library with spaces.jar", | ||
"library#version.jar", | ||
"[email protected]", | ||
"library&dependency.jar", | ||
"library+extra.jar", | ||
"library%encoded.jar" | ||
) | ||
|
||
testCases.foreach { jarName => | ||
val tempJar = createTestJarWithProperty(jarName, "test.properties", "version", "1.0.0") | ||
|
||
try { | ||
val result = readProperty(tempJar, "test.properties", "version") | ||
assertTrue(s"Should read property from JAR: $jarName", result.isDefined) | ||
assertEquals(s"Should get correct property value from: $jarName", "1.0.0", result.get) | ||
} finally { | ||
tempJar.toFile.delete() | ||
} | ||
} | ||
} | ||
|
||
@Test | ||
def testReadPropertyFromNormalJar(): Unit = { | ||
val jarName = "normal-library.jar" | ||
val tempJar = createTestJarWithProperty(jarName, "library.properties", "name", "scala-library") | ||
|
||
try { | ||
val result = readProperty(tempJar, "library.properties", "name") | ||
assertTrue("Should read property from normal JAR", result.isDefined) | ||
assertEquals("Should get correct property value", "scala-library", result.get) | ||
} finally { | ||
tempJar.toFile.delete() | ||
} | ||
} | ||
|
||
@Test | ||
def testReadPropertyNonExistentResource(): Unit = { | ||
val jarName = "test.jar" | ||
val tempJar = createTestJarWithProperty(jarName, "existing.properties", "key", "value") | ||
|
||
try { | ||
val result = readProperty(tempJar, "non-existent.properties", "key") | ||
assertFalse("Should return None for non-existent resource", result.isDefined) | ||
} finally { | ||
tempJar.toFile.delete() | ||
} | ||
} | ||
|
||
@Test | ||
def testReadPropertyNonExistentKey(): Unit = { | ||
val jarName = "test.jar" | ||
val tempJar = createTestJarWithProperty(jarName, "test.properties", "existing-key", "value") | ||
|
||
try { | ||
val result = readProperty(tempJar, "test.properties", "non-existent-key") | ||
assertFalse("Should return None for non-existent key", result.isDefined) | ||
} finally { | ||
tempJar.toFile.delete() | ||
} | ||
} | ||
|
||
private def createTestJarWithProperty(jarName: String, resourceName: String, propertyKey: String, propertyValue: String): Path = { | ||
val tempDir = Files.createTempDirectory("jar-encoding-test") | ||
val jarFile = tempDir.resolve(jarName) | ||
|
||
// Create the properties content | ||
val properties = new Properties() | ||
properties.setProperty(propertyKey, propertyValue) | ||
|
||
// Create a JAR file with the properties resource | ||
Using.resource(new java.util.jar.JarOutputStream(Files.newOutputStream(jarFile))) { jos => | ||
val entry = new java.util.jar.JarEntry(resourceName) | ||
jos.putNextEntry(entry) | ||
|
||
Using.resource(new java.io.ByteArrayOutputStream()) { baos => | ||
properties.store(baos, "Test properties") | ||
jos.write(baos.toByteArray) | ||
} | ||
|
||
jos.closeEntry() | ||
} | ||
|
||
jarFile | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ import org.jetbrains.plugins.scala.project._ | |
import org.jetbrains.plugins.scala.project.external.ScalaSdkUtils | ||
import org.jetbrains.plugins.scala.project.maven.ScalaMavenImporter._ | ||
|
||
import java.nio.file.Path | ||
import java.nio.file.{Path, Paths} | ||
import java.util | ||
import java.util.stream.Stream | ||
import scala.annotation.nowarn | ||
|
@@ -111,7 +111,9 @@ final class ScalaMavenImporter extends MavenApplicableConfigurator(PluginGroupId | |
val implicitScalaLibraryInfo = Option(mavenProject.getCachedValue(MavenImplicitScalaLibraryInfo)) | ||
implicitScalaLibraryInfo.map { info => | ||
val vfUrlManager = WorkspaceModel.getInstance(project).getVirtualFileUrlManager | ||
val jarUrl = vfUrlManager.getOrCreateFromUrl(s"jar://${info.path}!/") | ||
// Use proper URI encoding to handle special characters in JAR paths | ||
val jarUri = Paths.get(info.path).toUri.toString | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line doesn't compile as |
||
val jarUrl = vfUrlManager.getOrCreateFromUrl(s"jar://$jarUri!/") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making an uri from uri ?
|
||
|
||
val libraryRoot = new LibraryRoot(jarUrl, LibraryRootTypeIdCompanion.getCOMPILED, InclusionOptions.ROOT_ITSELF) | ||
storage.addLibraryEntity(info.libraryName, project, SerializationConstants.MAVEN_EXTERNAL_SOURCE_ID, Seq(libraryRoot)) | ||
|
131 changes: 131 additions & 0 deletions
131
scala/scala-impl/src/org/jetbrains/plugins/scala/project/SafeJarLoader.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
package org.jetbrains.plugins.scala.project | ||
|
||
import com.intellij.openapi.diagnostic.Logger | ||
import com.intellij.openapi.vfs.VirtualFile | ||
|
||
import java.io.File | ||
import java.net.{URL, URLClassLoader} | ||
import java.nio.file.{Files, Path, Paths} | ||
import scala.util.{Failure, Success, Try} | ||
|
||
/** | ||
* Utility for safely creating ClassLoaders and URLs from JAR files with proper URL encoding. | ||
* | ||
* This fixes SCL-24273: Classes decompiled from JAR files with special characters in their names are not resolved. | ||
* The root cause was manual construction of jar:file: URLs instead of using Java's proper Path→URI conversion APIs. | ||
* | ||
* Special characters that caused issues include: #, spaces, @, &, +, % | ||
*/ | ||
object SafeJarLoader { | ||
|
||
private val LOG = Logger.getInstance(getClass) | ||
|
||
/** | ||
* Creates a URLClassLoader from a JAR file with proper URL encoding. | ||
* | ||
* @param jarFile the JAR file to create a ClassLoader for | ||
* @param parent the parent ClassLoader (defaults to current ClassLoader) | ||
* @return Some(URLClassLoader) if successful, None if all attempts fail | ||
*/ | ||
def createClassLoader(jarFile: VirtualFile, parent: ClassLoader = null): Option[URLClassLoader] = { | ||
createJarUrl(jarFile.getPath).map { url => | ||
new URLClassLoader(Array(url), parent) | ||
} | ||
} | ||
|
||
/** | ||
* Creates a URLClassLoader from a JAR file path with proper URL encoding. | ||
* | ||
* @param jarPath the path to the JAR file | ||
* @param parent the parent ClassLoader (defaults to current ClassLoader) | ||
* @return Some(URLClassLoader) if successful, None if all attempts fail | ||
*/ | ||
def createClassLoader(jarPath: String, parent: ClassLoader = null): Option[URLClassLoader] = { | ||
createJarUrl(jarPath).map { url => | ||
new URLClassLoader(Array(url), parent) | ||
} | ||
} | ||
|
||
/** | ||
* Creates a proper JAR URL from a file path with correct encoding. | ||
* | ||
* This method tries multiple approaches to handle edge cases: | ||
* 1. Primary: Java NIO Path with proper URI encoding | ||
* 2. Fallback: Legacy File.toURI() method | ||
* | ||
* @param jarPath the path to the JAR file | ||
* @return Some(URL) if successful, None if all attempts fail | ||
*/ | ||
def createJarUrl(jarPath: String): Option[URL] = { | ||
val attempts = Seq( | ||
// Primary: NIO Path with proper URI encoding | ||
() => Paths.get(jarPath).toUri().toURL(), | ||
|
||
// Fallback: Legacy File.toURI() method for compatibility | ||
() => new File(jarPath).toURI().toURL() | ||
) | ||
|
||
attempts.view.flatMap { attempt => | ||
Try(attempt()) match { | ||
case Success(url) => | ||
LOG.debug(s"Successfully created URL for JAR: $jarPath -> $url") | ||
Some(url) | ||
case Failure(ex) => | ||
LOG.debug(s"URL creation attempt failed for $jarPath: ${ex.getMessage}") | ||
None | ||
} | ||
}.headOption.orElse { | ||
logJarLoadingIssue(jarPath, new IllegalStateException("All URL creation attempts failed")) | ||
None | ||
} | ||
} | ||
|
||
/** | ||
* Creates a JAR resource URL (jar:file://path!/resource) with proper encoding. | ||
* | ||
* @param jarPath path to the JAR file | ||
* @param resource resource path within the JAR (e.g., "META-INF/MANIFEST.MF") | ||
* @return Some(URL) if successful, None if creation fails | ||
*/ | ||
def createJarResourceUrl(jarPath: String, resource: String): Option[URL] = { | ||
createJarUrl(jarPath).flatMap { jarUrl => | ||
Try { | ||
// Use the properly encoded jar URL and append the resource | ||
new URL(s"jar:${jarUrl.toString}!/$resource") | ||
}.toOption.orElse { | ||
LOG.warn(s"Failed to create JAR resource URL for $jarPath!/$resource") | ||
None | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Creates a JAR resource URL from a URI string with proper encoding. | ||
* This is useful when you already have a file URI. | ||
* | ||
* @param jarFileUri the URI string of the JAR file | ||
* @param resource resource path within the JAR | ||
* @return Some(URL) if successful, None if creation fails | ||
*/ | ||
def createJarResourceUrlFromUri(jarFileUri: String, resource: String): Option[URL] = { | ||
Try { | ||
new URL(s"jar:$jarFileUri!/$resource") | ||
}.toOption.orElse { | ||
LOG.warn(s"Failed to create JAR resource URL from URI $jarFileUri!/$resource") | ||
None | ||
} | ||
} | ||
|
||
private def logJarLoadingIssue(jarPath: String, error: Throwable): Unit = { | ||
val fileName = Paths.get(jarPath).getFileName.toString | ||
val hasSpecialChars = """[#\s@&+%]""".r.findFirstIn(fileName).isDefined | ||
|
||
if (hasSpecialChars) { | ||
LOG.warn(s"JAR file name contains special characters that may cause URL encoding issues: $fileName") | ||
LOG.warn("Consider renaming the JAR file to avoid characters: # @ & + % (spaces)") | ||
LOG.warn("See: https://youtrack.jetbrains.com/issue/SCL-24273") | ||
} | ||
|
||
LOG.error(s"Failed to create URL for JAR: $jarPath", error) | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it?
This change is a refactoring (Extract variable + replace
format
with interpolated string). It hasn't change the behavior, right?