Skip to content

Commit 4192e5c

Browse files
fix: URI handling for URIs with special/blank characters [IDE-1203][IDE-1235] (#706)
* fix: improve path and URI conversion for cross-platform compatibility This commit enhances the path and URI conversion functionality to ensure reliable file path handling across different operating systems. Specifically improves Windows path handling with proper URI encoding/decoding and adds comprehensive tests to verify the bidirectional conversion between paths and URIs works correctly on both POSIX and Windows systems. * fix: more test cases and use ASCII strings * fix: toLanguageServerURL * fix: toLanguageServerURL tests * fix: improve language server initialization and URI handling [IDE-1203] - Increase language server initialization timeout from 20L to 2000L - Add proper error handling when adding content roots - Run content root addition asynchronously to prevent UI freezes - Improve error logging with more descriptive messages - Add user notification when language server fails to initialize - Replace size > 0 check with isNotEmpty() for better readability - Fix code formatting and parameter organization * fix: tests in ReferenceChooserDialogTest.kt * fix: error handling in fromUriToPath Co-authored-by: windsurf-bot[bot] <189301087+windsurf-bot[bot]@users.noreply.github.com> * fix: windsurf suggestion * fix: test setup for ReferenceChooserDialogTest.kt * chore: revert timeout * docs: CHANGELOG.md * fix: normalize path before using it to persist folderConfig * fix: Improve folder config path normalization and add tests [IDE-1203] Ensures that FolderConfigSettings consistently handles folder paths by normalizing and absolutizing them. The 'folderPath' property of FolderConfig objects managed by FolderConfigSettings will now always reflect this normalized, absolute path. This resolves an issue where the stored folderPath in FolderConfig objects did not always represent the fully normalized and absolute path used as the key in the settings map, leading to potential inconsistencies and failing tests for path normalization. Key changes include: - Added FolderConfigSettingsTest.kt with comprehensive unit tests for path normalization, covering various scenarios including paths with '.', '..', and equivalent path representations. - Converted tests to JUnit 4 syntax as per project standards. - Updated FolderConfigSettings: - 'addFolderConfig' now stores a copy of the FolderConfig with its 'folderPath' correctly normalized and absolutized. - 'createEmpty' now directly instantiates FolderConfig with the normalized path, improving clarity and efficiency. - Fixed a compile error in ReferenceChooserDialogTest.kt by refactoring a direct private field access to use a public method. * chore: update initialization messaging in summary [IDE-1235] * refactor: ReferenceChooserDialogTest.kt setup * refactor: add tests, normalize more paths * refactor: ensure file separator suffix in folder configs * fix: tests --------- Co-authored-by: Abdelrahman Shawki Hassan <[email protected]> (cherry picked from commit 9ba9809)
1 parent 807317a commit 4192e5c

File tree

13 files changed

+462
-148
lines changed

13 files changed

+462
-148
lines changed

.windsurfrules

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ always write and update test cases. iterate until they pass.
66
use existing mocks, don't write new ones.
77
if you use mocks, use mockk to generate them.
88
always run the tests after editing.
9+
use junit4 syntax
910

1011
** security **
1112
determine the absolute path of the project directory. you can do that e.g. by executing pwd on the shell within the directory.

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
# Snyk Security Changelog
22
## [2.13.1]
33
### Fixed
4-
- fixed not initialized exception in error handling during language server startup
4+
- fixed not initialized exception in error handling during language server startup
5+
- fixed handling of special characters in filepaths
56

67
## [2.13.0]
78

src/main/kotlin/io/snyk/plugin/Utils.kt

Lines changed: 10 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -421,12 +421,17 @@ fun String.toVirtualFile(): VirtualFile {
421421
return if (!this.startsWith("file:")) {
422422
StandardFileSystems.local().refreshAndFindFileByPath(this) ?: throw FileNotFoundException(this)
423423
} else {
424-
val filePath = Paths.get(this.toFilePathString())
424+
val filePath = fromUriToPath()
425425
VirtualFileManager.getInstance().refreshAndFindFileByNioPath(filePath)
426426
?: throw FileNotFoundException(this)
427427
}
428428
}
429429

430+
fun String.fromUriToPath(): Path {
431+
val filePath = Paths.get(URI.create(this))
432+
return filePath.normalize()
433+
}
434+
430435
fun String.toVirtualFileOrNull(): VirtualFile? {
431436
return try {
432437
this.toVirtualFile()
@@ -436,77 +441,11 @@ fun String.toVirtualFileOrNull(): VirtualFile? {
436441
}
437442

438443
fun VirtualFile.toLanguageServerURI(): String {
439-
return this.url.toFileURIString()
440-
}
441-
442-
/**
443-
* Normalizes a string that represents a file path or URI.
444-
*
445-
* This should be called on a string that represents an absolute path to a local file, or a file uri for a local file.
446-
* Relative paths and files on network shares are not currently supported.
447-
*
448-
* We deliberately avoid use of the Path, File and URI libraries as these will make decisions on how paths are handled
449-
* based on the underlying operating system. This approach provides consistency.
450-
*
451-
* @param forceUseForwardSlashes Whether to force the use of forward slashses as path separators, even for files on
452-
* Windows. Unix systems will always use forward slashes.
453-
* @return The normalized string.
454-
*/
455-
private fun String.toNormalizedFilePath(forceUseForwardSlashes: Boolean): String {
456-
val fileScheme = "file:"
457-
val windowsSeparator = "\\"
458-
val unixSeparator = "/"
459-
460-
// Strip the scheme and standardise separators on Unix for now.
461-
val normalizedPath = this.removePrefix(fileScheme).replace(windowsSeparator, unixSeparator)
462-
var targetSeparator = unixSeparator
463-
464-
// Split the path into parts, filtering out any blanks or references to the current directory.
465-
val parts = normalizedPath.split(unixSeparator).filter { it.isNotBlank() && it != "." }.mapIndexed { idx, value ->
466-
if (idx == 0) {
467-
// Since we only support local files, we can use the first element of the path can tell us whether we
468-
// are dealing with a Windows or unix file.
469-
if (value.startsWithWindowsDriveLetter()) {
470-
// Change to using Windows separators (if allowed) and capitalize the drive letter.
471-
if (!forceUseForwardSlashes) targetSeparator = windowsSeparator
472-
value.uppercase()
473-
} else {
474-
// On a Unix system, start with a slash representing root.
475-
unixSeparator + value
476-
}
477-
} else value
478-
}
479-
480-
// Removing any references to the parent directory (we have already removed references to the current directory).
481-
val stack = mutableListOf<String>()
482-
for (part in parts) {
483-
if (part == "..") {
484-
if (stack.isNotEmpty()) stack.removeAt(stack.size - 1)
485-
} else stack.add(part)
486-
}
487-
return stack.joinToString(targetSeparator)
488-
}
489-
490-
/**
491-
* Converts a string representing a file path to a normalised form. @see io.snyk.plugin.UtilsKt.toNormalizedFilePath
492-
*/
493-
fun String.toFilePathString(): String {
494-
return this.toNormalizedFilePath(forceUseForwardSlashes = false)
444+
return this.path.fromPathToUriString()
495445
}
496446

497-
/**
498-
* Converts a string representing a file path to a normalised form. @see io.snyk.plugin.UtilsKt.toNormalizedFilePath
499-
*/
500-
fun String.toFileURIString(): String {
501-
var pathString = this.toNormalizedFilePath(forceUseForwardSlashes = true)
502-
503-
// If we are handling a Windows path it may not have a leading slash, so add one.
504-
if (pathString.startsWithWindowsDriveLetter()) {
505-
pathString = "/$pathString"
506-
}
507-
508-
// Add a file scheme. We use two slashes as standard.
509-
return "file://$pathString"
447+
fun String.fromPathToUriString(): String {
448+
return Paths.get(this).normalize().toUri().toASCIIString()
510449
}
511450

512451
private fun String.startsWithWindowsDriveLetter(): Boolean {
@@ -517,7 +456,7 @@ fun VirtualFile.getDocument(): Document? = runReadAction { FileDocumentManager.g
517456

518457
fun Project.getContentRootPaths(): SortedSet<Path> {
519458
return getContentRootVirtualFiles()
520-
.mapNotNull { it.path.toNioPathOrNull() }
459+
.mapNotNull { it.path.toNioPathOrNull()?.normalize() }
521460
.toSortedSet()
522461
}
523462

src/main/kotlin/io/snyk/plugin/services/SnykTaskQueueService.kt

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import io.snyk.plugin.pluginSettings
2222
import io.snyk.plugin.refreshAnnotationsForOpenFiles
2323
import io.snyk.plugin.ui.SnykBalloonNotificationHelper
2424
import org.jetbrains.annotations.TestOnly
25+
import org.jetbrains.concurrency.runAsync
2526
import snyk.common.lsp.LanguageServerWrapper
2627
import snyk.common.lsp.ScanState
2728
import snyk.trust.confirmScanningAndSetWorkspaceTrustedStateIfNeeded
@@ -56,7 +57,13 @@ class SnykTaskQueueService(val project: Project) {
5657

5758
// wait for modules to be loaded and indexed so we can add all relevant content roots
5859
DumbService.getInstance(project).runWhenSmart {
59-
languageServerWrapper.addContentRoots(project)
60+
runAsync {
61+
try {
62+
languageServerWrapper.addContentRoots(project)
63+
} catch (e: RuntimeException) {
64+
logger.error("unable to add content roots for project $project", e)
65+
}
66+
}
6067
}
6168
}
6269

src/main/kotlin/io/snyk/plugin/ui/SnykSettingsDialog.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ class SnykSettingsDialog(
185185
// TODO: check for concrete project roots, and if we received a message for them
186186
// this is an edge case, when a project is opened after ls initialization and
187187
// preferences dialog is opened before ls sends the additional parameters
188-
additionalParametersTextField.isEnabled = LanguageServerWrapper.getInstance().folderConfigsRefreshed.isNotEmpty()
188+
additionalParametersTextField.isEnabled = LanguageServerWrapper.getInstance().getFolderConfigsRefreshed().isNotEmpty()
189189
additionalParametersTextField.text = getAdditionalParams(project)
190190
scanOnSaveCheckbox.isSelected = applicationSettings.scanOnSave
191191
cliReleaseChannelDropDown.selectedItem = applicationSettings.cliReleaseChannel

src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@ import com.intellij.openapi.project.ProjectManager
1212
import com.intellij.openapi.util.Disposer
1313
import com.intellij.openapi.util.io.toNioPathOrNull
1414
import com.intellij.openapi.vfs.VirtualFile
15+
import io.snyk.plugin.fromUriToPath
1516
import io.snyk.plugin.getCliFile
1617
import io.snyk.plugin.getContentRootVirtualFiles
1718
import io.snyk.plugin.getSnykTaskQueueService
1819
import io.snyk.plugin.getWaitForResultsTimeout
1920
import io.snyk.plugin.pluginSettings
2021
import io.snyk.plugin.runInBackground
21-
import io.snyk.plugin.toFilePathString
2222
import io.snyk.plugin.toLanguageServerURI
23+
import io.snyk.plugin.ui.SnykBalloonNotificationHelper
2324
import io.snyk.plugin.ui.toolwindow.SnykPluginDisposable
2425
import org.eclipse.lsp4j.ClientCapabilities
2526
import org.eclipse.lsp4j.ClientInfo
@@ -64,9 +65,9 @@ import snyk.common.lsp.commands.COMMAND_WORKSPACE_FOLDER_SCAN
6465
import snyk.common.lsp.commands.SNYK_GENERATE_ISSUE_DESCRIPTION
6566
import snyk.common.lsp.progress.ProgressManager
6667
import snyk.common.lsp.settings.FolderConfigSettings
68+
import snyk.common.lsp.settings.IssueViewOptions
6769
import snyk.common.lsp.settings.LanguageServerSettings
6870
import snyk.common.lsp.settings.SeverityFilter
69-
import snyk.common.lsp.settings.IssueViewOptions
7071
import snyk.common.removeTrailingSlashesIfPresent
7172
import snyk.pluginInfo
7273
import snyk.trust.WorkspaceTrustService
@@ -97,7 +98,7 @@ class LanguageServerWrapper(
9798

9899
// internal for test set up
99100
internal val configuredWorkspaceFolders: MutableSet<WorkspaceFolder> = Collections.synchronizedSet(mutableSetOf())
100-
internal var folderConfigsRefreshed: MutableMap<String, Boolean> = ConcurrentHashMap()
101+
private var folderConfigsRefreshed: MutableMap<String, Boolean> = ConcurrentHashMap()
101102
private var disposed = false
102103
get() {
103104
return ApplicationManager.getApplication().isDisposed || field
@@ -199,10 +200,10 @@ class LanguageServerWrapper(
199200
LanguageServerRestartListener.getInstance()
200201
refreshFeatureFlags()
201202
} else {
202-
logger.warn("Language Server initialization did not succeed")
203+
logger.error("Snyk Language Server process launch failed.")
203204
}
204205
} catch (e: Exception) {
205-
logger.warn(e)
206+
logger.error("Initialization of Snyk Language Server failed", e)
206207
if (processIsAlive()) process.destroyForcibly()
207208
isInitialized = false
208209
}
@@ -336,7 +337,7 @@ class LanguageServerWrapper(
336337
added.filter { !configuredWorkspaceFolders.contains(it) },
337338
removed.filter { configuredWorkspaceFolders.contains(it) },
338339
)
339-
if (params.event.added.size > 0 || params.event.removed.size > 0) {
340+
if (params.event.added.isNotEmpty() || params.event.removed.isNotEmpty()) {
340341
languageServer.workspaceService.didChangeWorkspaceFolders(params)
341342
configuredWorkspaceFolders.removeAll(removed)
342343
configuredWorkspaceFolders.addAll(added)
@@ -485,11 +486,12 @@ class LanguageServerWrapper(
485486
// the folderConfigs in language server
486487
val folderConfigs = configuredWorkspaceFolders
487488
.filter {
488-
val folderPath = it.uri.toFilePathString()
489+
val folderPath = it.uri.fromUriToPath().toString()
489490
folderConfigsRefreshed[folderPath] == true
490491
}.map {
491-
val folderPath = it.uri.toFilePathString()
492-
service<FolderConfigSettings>().getFolderConfig(folderPath) }
492+
val folderPath = it.uri.fromUriToPath().toString()
493+
service<FolderConfigSettings>().getFolderConfig(folderPath)
494+
}
493495
.toList()
494496

495497
return LanguageServerSettings(
@@ -621,7 +623,13 @@ class LanguageServerWrapper(
621623

622624
fun addContentRoots(project: Project) {
623625
if (disposed || project.isDisposed) return
624-
ensureLanguageServerInitialized()
626+
if (!ensureLanguageServerInitialized()) {
627+
SnykBalloonNotificationHelper.showWarn(
628+
"Unable to initialize the Snyk Language Server. The plugin will be non-functional.",
629+
project
630+
)
631+
return
632+
}
625633
ensureLanguageServerProtocolVersion(project)
626634
updateConfiguration(false)
627635
val added = getWorkspaceFoldersFromRoots(project)
@@ -665,7 +673,13 @@ class LanguageServerWrapper(
665673
executeCommand(param)
666674
}
667675

668-
fun sendSubmitIgnoreRequestCommand(workflow: String, issueId: String, ignoreType: String, ignoreReason: String, ignoreExpirationDate: String) {
676+
fun sendSubmitIgnoreRequestCommand(
677+
workflow: String,
678+
issueId: String,
679+
ignoreType: String,
680+
ignoreReason: String,
681+
ignoreExpirationDate: String
682+
) {
669683
if (!ensureLanguageServerInitialized()) throw RuntimeException("couldn't initialize language server")
670684
try {
671685
val param = ExecuteCommandParams()
@@ -753,6 +767,15 @@ class LanguageServerWrapper(
753767
shutdown()
754768
}
755769

770+
fun getFolderConfigsRefreshed(): Map<String?, Boolean?> {
771+
return Collections.unmodifiableMap(this.folderConfigsRefreshed)
772+
}
773+
774+
fun updateFolderConfigRefresh(folderPath: String, refreshed: Boolean) {
775+
val path = Paths.get(folderPath).normalize().toAbsolutePath().toString()
776+
this.folderConfigsRefreshed[path] = refreshed
777+
}
778+
756779

757780
companion object {
758781
private var instance: LanguageServerWrapper? = null

src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ class SnykLanguageClient :
199199
val service = service<FolderConfigSettings>()
200200
service.addAll(folderConfigs)
201201
folderConfigs.forEach {
202-
LanguageServerWrapper.getInstance().folderConfigsRefreshed[it.folderPath] = true
202+
LanguageServerWrapper.getInstance().updateFolderConfigRefresh(it.folderPath, true)
203203
}
204204
}
205205
}

src/main/kotlin/snyk/common/lsp/settings/FolderConfigSettings.kt

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ package snyk.common.lsp.settings
22

33
import com.intellij.openapi.components.Service
44
import com.intellij.openapi.project.Project
5+
import io.snyk.plugin.fromUriToPath
56
import io.snyk.plugin.getContentRootPaths
6-
import io.snyk.plugin.toFilePathString
7+
import io.snyk.plugin.suffixIfNot
78
import org.jetbrains.annotations.NotNull
89
import snyk.common.lsp.FolderConfig
910
import snyk.common.lsp.LanguageServerWrapper
11+
import java.io.File
12+
import java.nio.file.Paths
1013
import java.util.concurrent.ConcurrentHashMap
1114
import java.util.stream.Collectors
1215

@@ -17,19 +20,35 @@ class FolderConfigSettings {
1720

1821
@Suppress("UselessCallOnNotNull", "USELESS_ELVIS", "UNNECESSARY_SAFE_CALL", "RedundantSuppression")
1922
fun addFolderConfig(@NotNull folderConfig: FolderConfig) {
20-
if (folderConfig?.folderPath.isNullOrBlank() ?: true) return
21-
configs[folderConfig.folderPath] = folderConfig
23+
if (folderConfig.folderPath.isNullOrBlank()) return
24+
val normalizedAbsolutePath = normalizePath(folderConfig.folderPath)
25+
26+
val configToStore = folderConfig.copy(folderPath = normalizedAbsolutePath)
27+
configs[normalizedAbsolutePath] = configToStore
28+
}
29+
30+
private fun normalizePath(folderPath: String): String {
31+
val normalizedAbsolutePath =
32+
Paths.get(folderPath)
33+
.normalize()
34+
.toAbsolutePath()
35+
.toString()
36+
.suffixIfNot(File.separator)
37+
return normalizedAbsolutePath
2238
}
2339

2440
internal fun getFolderConfig(folderPath: String): FolderConfig {
25-
val folderConfig = configs[folderPath] ?: createEmpty(folderPath)
41+
val normalizedPath = normalizePath(folderPath)
42+
val folderConfig = configs[normalizedPath] ?: createEmpty(normalizedPath)
2643
return folderConfig
2744
}
2845

29-
private fun createEmpty(folderPath: String): FolderConfig {
30-
val folderConfig = FolderConfig(folderPath = folderPath, baseBranch = "main")
31-
addFolderConfig(folderConfig)
32-
return folderConfig
46+
private fun createEmpty(normalizedAbsolutePath: String): FolderConfig {
47+
val newConfig = FolderConfig(folderPath = normalizedAbsolutePath, baseBranch = "main")
48+
// Directly add to map, as addFolderConfig would re-normalize and copy, which is redundant here
49+
// since normalizedAbsolutePath is already what we want for the key and the object's path.
50+
configs[normalizedAbsolutePath] = newConfig
51+
return newConfig
3352
}
3453

3554
fun getAll(): Map<String, FolderConfig> {
@@ -58,7 +77,7 @@ class FolderConfigSettings {
5877
val additionalParameters = LanguageServerWrapper.getInstance().getWorkspaceFoldersFromRoots(project)
5978
.asSequence()
6079
.filter { LanguageServerWrapper.getInstance().configuredWorkspaceFolders.contains(it) }
61-
.map { getFolderConfig(it.uri.toFilePathString()) }
80+
.map { getFolderConfig(it.uri.fromUriToPath().toString()) }
6281
.filter { it.additionalParameters?.isNotEmpty() ?: false }
6382
.map { it.additionalParameters?.joinToString(" ") }
6483
.joinToString(" ")

src/main/kotlin/snyk/trust/TrustedProjects.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ package snyk.trust
44

55
import com.intellij.openapi.application.ApplicationManager
66
import com.intellij.openapi.application.invokeAndWaitIfNeeded
7-
import com.intellij.openapi.application.runInEdt
87
import com.intellij.openapi.components.service
98
import com.intellij.openapi.diagnostic.Logger
109
import com.intellij.openapi.project.Project

src/main/resources/html/ScanSummaryInit.html

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,10 @@
3939
${ideStyle}
4040
</head>
4141
<body>
42-
<div class="snx-header">
43-
<h1 class="snx-title snx-h1">Snyk Security is loading...</h1>
44-
</div>
4542
<div class="snx-summary">
4643
<div class="snx-status">
4744
<span class="snx-loader"></span>
48-
<p class="snx-message">Waiting for the Snyk CLI to be downloaded and the Language Server to be initialized. </p>
45+
<p class="snx-message">Snyk Security is loading...</p>
4946
</div>
5047
</div>
5148
</body>

0 commit comments

Comments
 (0)