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

Improper LSP URI escaping leads to RuntimeErrors when Diagrams are opened in KEITH #128

Open
Rojods opened this issue Jul 1, 2022 · 3 comments
Assignees
Labels
bug Something isn't working duplicate This issue or pull request already exists question Further information is requested requires client change This change is linked to some change on the client side implementation in klighd-vscode

Comments

@Rojods
Copy link

Rojods commented Jul 1, 2022

Hello all! As my first issue here, I'd like to thank you for the nice ecosystem you're building here, specially for visualization. Fantastic work :)!

For some contextualization on this issue, I'm the creator and current maintainer of ForSyDe IO. For my PhD research, I just developed a quick outputer of my analysis/synthesis to KGT since I haven't found the actual jars from KlighD (or similar) in maven central. All seemed to be pretty printed nicely and then I downloaded KEITH 0.1.0 nightly for Windows, since I'm running it on my machine.

[Problem]
Once I tried opening the diagram (fairly big one, with 1000+ lines of code), I got two runtime errors related to URISyntaxException. I thought that I had messed up something, so I tried checking a subset of my output, until I was sure it was indeed a valid kgt file. After digging in the error itself, I finalized realized the problem was with how the LSP server loads and represents the files from disk: my Windows machine represents the file path like file:///C:/Users/My Name With Space/..../file.kth, which cannot be put verbatim as a URI/URL. Sadly, that's exactly what the LSP seems to be doing according to lines 561-580 of package de.cau.cs.kieler.klighd.lsp.KGraphLanguageServerExtension .

Now, I could be wrong, but the root of the problems seems to be that the xtext extensions, or the JVM by association, does not encode/escape special characters in a string when you ask it to. In my case, since the user has spaces in it, the LSP complains exactly at the spot where my username has an space. In any case, the result is that I get no diagram and a big red RuntimeException in my beautiful UI.

There's another clear evidence that the problem is indeed special characters in the path during encoding: I copied the KGT file into another folder which has no spaces in its path (Users\Public, which I guess exists by default in most Windows machines), and KEITH did its drawing job nicely without hiccups.

[Suggestion]
I think an easy fix is to just replace the offending characters, which won't require any additional effort from extra libraries and others. But wouldn't it be more future-proof if a library (e.g. Guava) is used to encode the URL and "outsource" this problem to more mature libraries? Just giving my 2 cents :).

@NiklasRentzCAU
Copy link
Member

Hey there!
First of all, thanks for the nice words! This is one of the frameworks we use for our research ourselves heavily and therefore try to improve it in multiple ways along the way as well.

For your questions/problems:
Yes, you cannot find jars on Maven Central (yet), as the Eclipse-version is currently only available as a P2 update site, with the newest release hosted here. As we want to further push the web version that you seem to be using a version of we will also try to get this server and support for languages more easily accessible as dependencies with an easy API on Maven. For now, a pre-built language server such as from the nightly build of our semantics can run some of our languages, including the kgt language. This can be used either in combination with our vs code extension or the web-based CLI tool, the KEITH tool itself (based on Theia) is kinda deprecated, as the VS Code support includes that as well.

The issue with whitespaces is already known and tracked over here. As the comment over there states, the issue may be unavoidable by us and caused by one of our dependencies. I will have a look into that to see if we can fix that anyways. As you already noticed, for now a workaround of not using paths with spaces will work just fine.

@NiklasRentzCAU NiklasRentzCAU added bug Something isn't working duplicate This issue or pull request already exists question Further information is requested requires client change This change is linked to some change on the client side implementation in klighd-vscode labels Jul 5, 2022
@NiklasRentzCAU NiklasRentzCAU self-assigned this Jul 5, 2022
@Rojods
Copy link
Author

Rojods commented Jul 7, 2022

Thanks a lot for the quick response @NiklasRentzCAU! I agree it's a duplicate, but i won't close it already as duplicate since the issue is in another repo! But If you think it's okay to close it, then I do it happily.

For future reference, which channel do I use to ask conceptual questions regarding klighd, to avoid clogging the issue tracker?

@NiklasRentzCAU
Copy link
Member

You can either contact me via mail directly or use the discussions tab provided by GitHub for exactly such Q&A, that should be enabled now for this repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists question Further information is requested requires client change This change is linked to some change on the client side implementation in klighd-vscode
Projects
None yet
Development

No branches or pull requests

2 participants