search

eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
19.98k stars 2.5k forks source link

Absolute paths probably don't work for absolute icons paths #8434

Open tsmaeder opened 4 years ago

tsmaeder commented 4 years ago

Bug Description:

While debugging something, I ended up with an icon url (in a TreeItem, to be precise) that reference a file outside of the plugin package directory. So I ended up with a URI for the resource like this: /hostedPlugin/pluginId/../foo/bar/icon.png. When the back end was trying to resolve this resource, the file it was trying to load was: <plugin package for plugin 'foo'>/bar/icon.png it seems like the .. gets resolved as "remove the part pluginId from the path" somewhere along the ways.

I currently don't have time to debug this properly, hence this bug report.

Steps to Reproduce:

Make a plugin that registers a tree view with an icon that you load from the user home directory, instead of relative to the plugin.

akosyakov commented 4 years ago

Do you have a plugin to reproduce? I can imagine someone can write such paths in package.json, but I am not sure whether it would be correct.

tsmaeder commented 4 years ago

vscode-java-test uses absolute paths for icons in tree items. It's how I observed the problem: https://github.com/microsoft/vscode-java-test/blob/master/src/explorer/testExplorer.ts#L104 Whether this would be valid in a package.json, I don't know at this time.

tsmaeder commented 4 years ago

There is an api that even mandates absolute paths: see gutterIconPath in https://code.visualstudio.com/api/references/vscode-api#DecorationRenderOptions As for the use-case for non-package local icons, i can think of extensions sharing resources (../otherPlugin/icons/foo.png) or extensions using resources from a program installed in the system. I don't think it's a very common case, though.

akosyakov commented 4 years ago

ok, it looks valid, thanks for checking. For decorations we already using file endpoint: https://github.com/eclipse-theia/theia/blob/e36195e0cd90ecb17db4e26ae29ca430d35640db/packages/plugin-ext/src/main/browser/text-editors-main.ts#L161-L166 Probably we should use for absolute paths in other places.

tsmaeder commented 4 years ago

Well...we need to keep in mind that plugin hosts may run on machines different from the back end machine. So it won't be that simple, unfortunately.

akosyakov commented 4 years ago

In Theia it is fine to make it simple, in downstream product someone can override file endpoint and make it complicated. No need to pull complexity to upstream

tsmaeder commented 4 years ago

Just saying we should not hard-code this in places it can't be customized by framework adopters.