-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix: do not reject unknown schemes in WindowStateExt.asExternalUri #13057
fix: do not reject unknown schemes in WindowStateExt.asExternalUri #13057
Conversation
c341da0
to
3cebec6
Compare
@AlexandraBuzila Is this ready for a re-review? |
The "Connect to Github..." workflow still does not work for me: when I invoke it, nothing happens and I get a dialog saying
I would expect to be redirected to the github authentication page. |
Does the link open in the browser if you confirm that dialog? I see now that the workflow in vscode looks different: you are asked to log in, while in theia you are asked to use another login method. I can check to see where the difference is coming from. |
@tsmaeder Opne question above :-) |
No |
3cebec6
to
43ca34a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After retesting, the login seems to work now.
Fixes #128210 Contributed on behalf of STMicroelectronics Signed-off-by: Alexandra Buzila <abuzila@eclipsesource.com>
43ca34a
to
d7dbfe4
Compare
I'm very much looking forward to having this up and running. So I tried with today's master, removed the git and github related extensions from No browser window was opened. Do you have any suggestion @AlexandraBuzila? Thank you! |
Joining in as I too am very interested in this feature and I stop at the same issue as the post above. |
@AlexandraBuzila @tsmaeder Do you have feedback on the issue mentioned above? Should we re-open this issue? |
@AndKe how exactly is this failing for you? What's your setup for running Theia? |
@tsmaeder - I recieved good help from @planger to get this far: (so our methods are most likely more or less the same) I started with copying my working plugins from vscode: What I got was : It may be worth mentioning that my copilot for vscode authentication also failed to spawn a browser window, like Arduino IDE fell back to alternative method. |
@AndKe arduinoIDE is still on Theia release 1.41.0. The fix is not in that version. |
@tsmaeder are you saying that is should work in >= 1.41 ? / and where can I get that version? |
@AndKe you must have forgotten the details of the discussion we had about this over on Arduino Forum. The "arduino-ide_2.3.3-snapshot-de5b91f_Linux_64bit.AppImage" you are using is a beta tester build of Arduino IDE with Theia version 1.46.1 (arduino/arduino-ide@de5b91f): https://github.com/arduino/arduino-ide/actions/runs/8596568336#artifacts |
@AndKe as the PR was merged in Dec., it should be in 1.46.1.
This is not a good idea: when we download plugins, we select a version of the built-ins that works with that particular Theia version. As for the gitlens example from the testing instructions, I don't even get the "Connect to Github" link anymore. Reopening. |
What it does
Fixes #12821
After testing the same workflow of the Gitlens extension in VS Code, I think URLs with unknown schemes were rejected prematurely in Theia.
When the
Connect to Github
action is triggered, the extension invokes two methods of theWindowStateExtImpl
: first asExternalUri and at a later point openUri.The parameter passed to the
asExternalUri
method is an URI of the formtheia://...
and the existing code rejects it.In VS Code, the URI has an application specific schema as well (not http or https), but the code does not reject it. This allows the Github authentication to process the request and later on call the
openUri
method with a valid URI.I modified the code and it now handles URIs similar to VS Code. This fixes the initial problem, which was reported for the gitlens extension, but I would love some feedback as I can't tell if this is generally safe for all extensions.
How to test
Review checklist
Reminder for reviewers
Contributed on behalf of STMicroelectronics