Brackets: User is able to create file/folders using relative path

[sdalmeida]

Prerequisites

• [x] Can you reproduce the problem with Debug -> Reload Without Extensions?
• [x] Did you perform a cursory search to see if your bug or enhancement is already reported?
• [x] Did you read the Troubleshooting guide?

Description

This bug was first found by @pomax(mozilla#434) and then fixed by @tallandroid (#11862).

"It's possible to right-click in the file tree and create a file (or folder) name it ../something, and the path isn't resolved, but rather the .. and / characters are treated as literals to be used in the filename itself." - @humphd(#11609)

It looks like this bug wasn't fully fixed. I'm using the latest version of Brackets and I'm still able to reproduce it.

Steps to Reproduce

1. Create/Rename a file
2. Give it a valid relative path with a unique file name (Eg. ../../file1)
3. Save it
4. File is now created in whatever path you gave it

Expected behavior: An error message should show complaining about the relative path.

Actual behavior: File is renamed/created without an error message.

Versions

OS: macOS sierra 11.2.3
Brackets Version: 1.8 build 1.8.0-17108

[ficristo]

Do you want to take a look at this?

[tallandroid]

@simon66 Thanks for bringing it up. I can take a look into this later this week. If you want to pick it up before that , feel free to pick it up.

[Pomax]

@simon66 is a student currently helping us (the Mozilla Foundation) run through a bug list for https://thimble.mozilla.org, which relies on brackets for its code editor part (it's on 1.4 at the moment but we're uplifting to 1.8), finding and fixing issues that are good first bugs for first time contributors. In this particular case, unless simon wants to dive very deep, I'd recommend having someone more knowledgeable w.r.t. the Brackets codebase look at this particular issue =)

[humphd]

I think it's fine if he wants to try it. @simon66 I'd suggest beginning with converting your STR above to a failing test case or two in https://github.com/tallandroid/brackets/blob/master/test/spec/ProjectModel-test.js, and once you can get your tests to fail, move on to a fix.

[sdalmeida]

@ficristo Yep, I'd like to fix this issue, I'll take @humphd approach and once everything is done, I'll do a PR :+1:
I'll have my PR later on this week :)

[sdalmeida]

Hi @humphd
Looking at the test files, I noticed that there are a collection of file tests already in https://github.com/tallandroid/brackets/blob/master/test/spec/FileSystem-test.js
After running Jasmine Spec runner on "FileSystem", it shows that the test passed.
Where should the test case go? Under FileSystem-test.js or ProjectModel-test?
Thanks :)

[zaggino]

@simon66
Where you put the tests depends mostly on where you fix the code. If you fix the code in FileSystem.js you'd put the tests in FileSystem-test.js, when you fix ProjectModel.js you'd put that into ProjectModel-test.js. It's a rough guideline and of course there are exceptions to this but you should see what the other tests are testing and see where your test fits the best. Since this is a file tree issue, I'd also look if your test doesn't fit into FileTreeView-test.js suite.

[sdalmeida]

just wanted to update my progress on this bug. My main development unit died last week and I'm waiting for it to be repaired (I should've backed up my data haha). I should be getting it back today. I'll continue working on it as soon as I get my computer back :)

Also, thanks @zaggino for letting me know where I can put my test cases 👍

[ficristo]

@simon66 thank you for the update!

[Pomax]

@simon66 that's the nice thing about github forks: remember to push to your repo often, even if your code is "not ready for a PR" yet - you can't lose work that way, even if your computer catches fire =)

(although hopefully of course they were able to fix your machine with all the data intact)

[sdalmeida]

Hey guys! I'm back from the "dead" haha
Wow! What a headache :/ Lesson learned, always "push" your modifications.

Going back to the code, why is the invalid chars different based on the OS?

Why is < and > valid on MacOS and Linux but invalid on Windows? Shouldn't they be invalid for every OS?

Also, the reason why this bug still exists is because .. is not being checked.

These are the chars that I believe should be invalid:

• \?
• \\
• \*
• \.{2,} <-- .. or more should not be allowed, but allow .
• \.$ <-- allow a file to have . but not end with .
• \/
• \|
• \>
• \<

Are there any others chars that should be added/removed?

Also, my regex is not so good, but this is the pattern that I wrote:
([\?\*\|\:\/\<\>]+|\.{2,}|\.$) <-- I know I don't have to escape every char but for consistency I always do :)

Any help on writing a better pattern? What do you think?
Thanks :)

[zaggino]

Hi, I agree the list should be same for every OS as generally you don't want someone to push an invalid file to a project which other developer on different OS can't use.