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

Allow configurable viewer URL #2341

Closed

Conversation

lukas-hetzenecker
Copy link

this supports e.g. LaTeX-Workshop running in code-server

I know similar things were previously discussed - e.g. in #2326, #1265, #1271
I am not sure what the original PR in #1271 was as it seems to have been force-pushed to remove the configurable URL, but the approach here should have minimal security implications.


LaTeX-Workshop is already working reasonably well with code-server, with most of the features working out-of-box.
Only the integrated PDF viewer does not - for the simple reason that its iframe is loaded from http://localhost:random_port.

This pull request changes nothing in terms of security (e.g. embedded http server is still only bound to 127.0.0.1).
The only change is that the viewer can be loaded from a different URL.
Then the reverse proxy feature of code-server can be leveraged for accessing the viewer.

Example

latex-workshop.viewer.pdf.internal.url is set to https://%p.codeserver.com
and code-server started with code-server --proxy-domain codeserver.com

This request gets now sent to code-server, which proxies it to localhost:%p. All security implications of that are outsourced to code-server (and its proxy-domain feature) and/or the loadbalancer before.

Please let me know if you are willing to accept such a feature request.

this supports e.g. LaTeX-Workshop running in code-server
@tamuratak
Copy link
Contributor

Out of the scope of the extension.

@tamuratak tamuratak closed this Oct 25, 2020
Repository owner locked as resolved and limited conversation to collaborators Oct 28, 2020
Repository owner unlocked this conversation Jan 2, 2023
@James-Yu James-Yu reopened this Jan 2, 2023
@James-Yu James-Yu self-assigned this Jan 2, 2023
@James-Yu
Copy link
Owner

James-Yu commented Jan 3, 2023

@modaresimr May I invite you to introduce your steps of usage of this patch on code-server, so that the extension will work? Is the proposed port placeholder %p a must?

@modaresimr
Copy link
Contributor

%p placeholder is a need since we need to be able to distinguish between different ports in vscode server. For example, when we open two repositories we will have two ports.

based on the documentation there is two ways to access internal ports in vscode server: (subdomain) and (path) https://coder.com/docs/code-server/latest/guide#accessing-web-services

However, the problem is sth else:

Latex-Workshop viewer uses an absolute path (starts with /).
However, VSCode server makes a proxy for an internal port to a path like /proxy/port/
It means that if my vscode server URL is vscode.mydomain.com, and the port for the pdf viewer is 1000, the viewer URL can be vscode.mydomain.com/proxy/1000/
Therefore, vscode.mydomain.com/proxy/1000/viewer.html can be opened successfully. However, since its resources like js files have absolute paths, the browser tries to open vscode.mydomain.com/*. for example, for /viewer.js it will open vscode.mydomain.com/viewer.js instead of vscode.mydomain.com/proxy/1000/viewer.js

This is why I use https://%p.vscode.mydomain.com in my fork

However, I think https://vscode.mydomain.com/proxy/%p/ is a better option

@James-Yu
Copy link
Owner

James-Yu commented Jan 3, 2023

Thanks @modaresimr for your very detailed explanation! May I interpret the message as that, as long as absolute paths are changed to relatives, the viewer will work for code-server without the need of customizing localhost:port? Or the latter is a must and prerequisite?

@modaresimr
Copy link
Contributor

From my point of view, there is no need.
Simply, if you return this URL: window.location.protocol+"//"+window.location.host+"/proxy/{{port}}/viewer.html?file=...., the pdf viewer should work (after removing the / at the beginning of resources).

Normally, asExternalUri should do the same thing but in reality, it will not add the path to it (it returns only myserver.com/proxy/port/). Therefore, we need to handle it by ourselves.

@James-Yu
Copy link
Owner

James-Yu commented Jan 4, 2023

I'm inclined on not touching the server url part as it is highly error-prone. As I do not use code-server, a PR on this config item change is welcomed and I will review it.

@James-Yu James-Yu closed this Jan 4, 2023
@modaresimr
Copy link
Contributor

I have fixed the problem in #3639

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants