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

URI.allLocations leads to endless loop if URI has own schema #6377

Closed
corneliusludmann opened this issue Oct 12, 2019 · 1 comment · Fixed by #6378
Closed

URI.allLocations leads to endless loop if URI has own schema #6377

corneliusludmann opened this issue Oct 12, 2019 · 1 comment · Fixed by #6378
Assignees
Labels
bug bugs found in the application

Comments

@corneliusludmann
Copy link
Contributor

corneliusludmann commented Oct 12, 2019

Description

When you create an URI object with an individual scheme (not file:), calling .allLocations leads to an endless loop. (Creating an URI with new URI('foo.txt') assumes an absolute path /foo.txt which prevents the problem described below.) An URI with own scheme is used in user-preferences-provider.ts, for example.

Reproduction Steps

Execute the following code:

const allLocations = new URI().withScheme('foo').withPath('foobar.txt').allLocations;

will results in an endless loop at this code position: https://github.com/eclipse-theia/theia/blob/master/packages/core/src/common/uri.ts#L54

let location: URI = this;
while (!location.path.isRoot) {
    locations.push(location);
    location = location.parent;
}

The reason is that location is equal to location.parent and location.path.isRoot === false.

In my opinion, the root of this problems lies in Path.dir which returns this when there is no path separator (see https://github.com/eclipse-theia/theia/blob/master/packages/core/src/common/path.ts#L124). I think, it would be more intuitive if undefined or so will returned in this case. Or we need something like hasParent to distinguish this situation because the isRoot check is not sufficient here.

To fix this with as little potential side effects as possible, I suggest changing the while condition as follows:

let location: URI = this;
while (!location.path.isRoot && location.toString() !== location.parent.toString()) {
    locations.push(location);
    location = location.parent;
}
@corneliusludmann
Copy link
Contributor Author

Added PR #6378 that implements the Path.hasParent solution.

@akosyakov akosyakov added the bug bugs found in the application label Oct 13, 2019
corneliusludmann added a commit to corneliusludmann/theia that referenced this issue Oct 14, 2019
…ith no parent

- Adds `Path#hasDir`
- Adds `location.path.hasDir` check in while loop of `URI#allLocations`
- Adds some tests

Fixes eclipse-theia#6377

Signed-off-by: Cornelius A. Ludmann <[email protected]>
akosyakov pushed a commit that referenced this issue Oct 16, 2019
- Adds `Path#hasDir`
- Adds `location.path.hasDir` check in while loop of `URI#allLocations`
- Adds some tests

Fixes #6377

Signed-off-by: Cornelius A. Ludmann <[email protected]>
akosyakov pushed a commit to akosyakov/theia that referenced this issue Feb 24, 2020
…ith no parent

- Adds `Path#hasDir`
- Adds `location.path.hasDir` check in while loop of `URI#allLocations`
- Adds some tests

Fixes eclipse-theia#6377

Signed-off-by: Cornelius A. Ludmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants