Skip to content
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

[pkg:dtd] DTD filesystem checks should be case-insensitive for drive letters on Windows #54917

Closed
DanTup opened this issue Feb 14, 2024 · 0 comments
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams.

Comments

@DanTup
Copy link
Collaborator

DanTup commented Feb 14, 2024

We've had a number of bugs in the past caused by drive letters sometimes being uppercase and sometimes being lowercase (and code that treats all paths case-sensitively). The casing of drive letters is somewhat inconsistent depending on the source of the path. For example VS Code's debugger and terminal normalise drive letters to uppercase, but it's URI class normalises them to lowercase (😔).

While in general we should consider case-sensitivity of entire paths, it's often hard to do with URIs because it's not guaranteed we even have access to the underlying volume to know if it's case sensitive (and while VS Code does it, it's not sound to assume Windows == case-insensitive and others == case sensitive, because you can have a mix of each on both Windows and macOS).

To avoid some issues, we tend to normalize all drive letters to uppercase in some places (for ex. the VS Code extension, analysis server, DAP). I think we should do the same with DTD's file system access because the workspace roots are set by the IDE (which might normalise things one way) and requests for the files may come from a completely different source (such as a DevTools extension).

Currently, using the wrong drive letter casing results in permission denied.

==> {
	"id": "3",
	"jsonrpc": "2.0",
	"method": "FileSystem.readFileAsString",
	"params": {
		"uri": "file:///c%3A/Dev/Dart-Code/Dart-Code/src/test/test_projects/hello_world/bin/main.dart"
	}
}
<== {
	"jsonrpc": "2.0",
	"result": {
		"type": "FileContent",
		"content": "import 'dart:async';....."
	},
	"id": "3"
}

==> {
	"id": "4",
	"jsonrpc": "2.0",
	"method": "FileSystem.readFileAsString",
	"params": {
		"uri": "file:///C:/Dev/Dart-Code/Dart-Code/src/test/test_projects/hello_world/bin/main.dart"
	}
}
<== {
	"jsonrpc": "2.0",
	"error": {
		"code": 142,
		"message": "Permission denied",
		"data": {
			"request": {
				"id": "4",
				"jsonrpc": "2.0",
				"method": "FileSystem.readFileAsString",
				"params": {
					"uri": "file:///C:/Dev/Dart-Code/Dart-Code/src/test/test_projects/hello_world/bin/main.dart"
				}
			}
		}
	},
	"id": "4"
}

@CoderDake

@keertip keertip changed the title [pkg:dtd] DTD filesystem checks should be case-insensitive (at least for drive letters) on Windows [pkg:dtd] DTD filesystem checks should be case-insensitive for drive letters on Windows Feb 15, 2024
@keertip keertip added the area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. label Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams.
Projects
None yet
Development

No branches or pull requests

2 participants