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

Converting a an Url to a PathBuf and back to Url won't give the same result #150

Open
icsaszar opened this issue Mar 8, 2020 · 2 comments

Comments

@icsaszar
Copy link

icsaszar commented Mar 8, 2020

I know Url is just reexported by lsp-types but I think the issue is more relevant to this crate than to the url crate (since it affects this crate's users), but I'll be happy to open an issue there too if this is not the right place.

I'm working on a language server project and with an extension for visual stdio code and ran into the following issue:

I store my files in a DashMap<Url, MyFile> since most of the time the lookups are done based on requests and notifications from the client, and converting each Url to a PathBuf would be quite wasteful. I also scan the opened folder (based on RootUri) on startup to construct the whole database of files. When scanning I convert the RootUri to a PathBuf, collect the files from the directory and convert each PathBuf to an Url to insert it in the map (I think this is acceptable since it's only done once).

But when I tried to check if a file that is being opened (didOpen) is in the DashMap after running the initial scanning, the lookup failed.

The reason is the visual studio code sends the uri with a %3A instead of : after the disk letter on windows. Link to playground.

use url;

#[cfg(test)]
mod test {
    #[test]
    fn test() {
        let url_from_vscode = url::Url::parse("file:///C%3A/test.txt").unwrap();
        let path = url_from_vscode.to_file_path().unwrap();
        let url_from_path = url::Url::from_file_path(&path).unwrap();
        println!("{:?}", &url_from_vscode);
        println!("{:?}", &path);
        println!("{:?}", &url_from_path);
        assert!(url_from_vscode == url_from_path);
    }
}

Thanks in advance for the help and keep up the great work.

@Marwes
Copy link
Member

Marwes commented Mar 8, 2020

Is there something actionable on the crate side? You should be able to normalize the Url before using it as a key for the map. Maybe we could do this when deserializing but that seems potentially surprising also.

@icsaszar
Copy link
Author

icsaszar commented Mar 8, 2020

According to this issue, they don't plan on providing canonicalization.

The other option seems to be url_normalizer, but I haven't tested it.

The simplest thing your crate could do is have a warning in the docs that url normalization is not handled and urls coming from clients might not match urls that the Url produces (using from_file_path()).

The other option would be to wrap Url and provide an Eq implementationt that normalizes both urls before checking.

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

No branches or pull requests

2 participants