OmniSharp / omnisharp-roslyn

OmniSharp server (HTTP, STDIO) based on Roslyn workspaces
MIT License
1.77k stars 421 forks source link

[LSP] Language server returns improper URIs #1125

Open LoneBoco opened 6 years ago

LoneBoco commented 6 years ago

omnisharp-roslyn is returning improperly formatted uris to requests.

{
    'diagnostics': [
        {
            'code': 0,
            'message': 'Unnecessary using directive.',
            'range': {
                'end': {'character': 37, 'line': 3},
                'start': {'character': 0, 'line': 3}
            },
            'severity': 4,
            'source': 'csharp'
        }
    ],
    'uri': 'file:///C%3A/file.cs'
}

The 'uri' encodes the :, which it should not do. The : should never be encoded and this will break software that tries to decode the uri. For example, this is what happens in Python:

>>> urllib.parse.urlparse('file:///C%3A/file.cs').path
'/C%3A/file.cs'
>>> urllib.parse.urlparse('file:///C:/file.cs').path
'/C:/file.cs'
>>> urllib.request.url2pathname('/C%3A/file.cs')
'\\C:\\file.cs'
>>> urllib.request.url2pathname('/C:/file.cs')
'C:\\file.cs'

Using urlparse and url2pathname are considered the correct way to handle uris in Python and it completely chokes on an improper uri.

LoneBoco commented 6 years ago

It appears this function is the culprit: https://github.com/OmniSharp/omnisharp-roslyn/blob/e7dc6255cae7b6169c56d76387f36f6add3774dd/src/OmniSharp.LanguageServerProtocol/Helpers.cs#L63-L68

Now, the language server spec doesn't really say anything about how to encode uris. It looks like VSCode is using %3A instead of :, so is this a workaround for VSCode? Should clients be built to un-mangle %3A encoding?

david-driscoll commented 6 years ago

We're just following the vscode way because without it, if I recall correctly it falls over.

Looks like there might be a workaround on the vscode side, so we can look at changing it to be more correct.

https://github.com/Microsoft/vscode-languageserver-node/issues/105