Skip to content

Create a URI from a file system path#40

Open
vfonic wants to merge 1 commit intofabiospampinato:masterfrom
vfonic:fix-file-uri-parsing
Open

Create a URI from a file system path#40
vfonic wants to merge 1 commit intofabiospampinato:masterfrom
vfonic:fix-file-uri-parsing

Conversation

@vfonic
Copy link

@vfonic vfonic commented May 19, 2019

Closes #39

  • Instead of allowing parsing arbitrary URI schemas

The issue is due to Uri.parse allowing any kind of URI, from 'untitled' to 'http' to 'file' to 'vscode-resource' (this one should be used by Webview to restrict access to only certain files/paths).

The above fix explicitly parses path using a file schema.

The best documentation I found on this was reading comments in the vscode.d.ts file.

* Instead of allowing parsing arbitrary URI schemas
@vfonic vfonic changed the title Create an URI from a file system path Create a URI from a file system path May 19, 2019
@fabiospampinato
Copy link
Owner

The root issue seems to be that this function:

getRootPath ( basePath? ) {
const {workspaceFolders} = vscode.workspace;
if ( !workspaceFolders ) return;
const firstRootPath = workspaceFolders[0].uri.fsPath;
if ( !basePath || !absolute ( basePath ) ) return firstRootPath;
const rootPaths = workspaceFolders.map ( folder => folder.uri.fsPath ),
sortedRootPaths = _.sortBy ( rootPaths, [path => path.length] ).reverse (); // In order to get the closest root
return sortedRootPaths.find ( rootPath => basePath.startsWith ( rootPath ) );
},

isn't returning an actual file system path sometimes, I think the fix should be instead to return undefined under those circumstances.

@vfonic
Copy link
Author

vfonic commented May 20, 2019

Hmmm, I'm not sure about that, that might be a separate issue. With the fix I provided here, I don't see the error message appearing on my machine anymore.

@fabiospampinato
Copy link
Owner

fabiospampinato commented May 21, 2019

Hmmm, I'm not sure about that, that might be a separate issue.

Since the value that's getting passed to URI.parse is being generated from there I think it'd be better to fix the root source of the issue.

@vfonic
Copy link
Author

vfonic commented May 21, 2019

Are you sure that's what's happening? Maybe it's just that VSCode update introduced throwing these errors (warnings?) and your code works as good as it used to?

I think using URI.parse might be problematic in itself since you wouldn't want someone to pass in a web URI instead of file and it still resolves correctly, or?
So, in that, maybe this can be a separate fix. With this change, I'm not seeing any errors anymore. I'm not sure what errors you're getting and if those are the same.

@fabiospampinato
Copy link
Owner

Are you sure that's what's happening? Maybe it's just that VSCode update introduced throwing these errors (warnings?) and your code works as good as it used to?

I think so, you can see from here where the value that's getting passed to URI.parse comes from.

I think using URI.parse might be problematic in itself since you wouldn't want someone to pass in a web URI instead of file and it still resolves correctly, or?

Maybe, realistically I don't see anyone manually adding some arbitrary path to their configuration though.

So, in that, maybe this can be a separate fix.

🤔 I see that function is being called also from here.

I'll have to take a closer look at this 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: BAD uri lacks scheme, falling back to file-scheme.

2 participants