This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Repository navigation
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
User is able to create file/folders using relative path #13099
Description
Prerequisites
- Can you reproduce the problem with
Debug -> Reload Without Extensions? - Did you perform a cursory search to see if your bug or enhancement is already reported?
- 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
- Create/Rename a file
- Give it a valid relative path with a unique file name (Eg.
../../file1) - Save it
- 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
Reacted by Pete Nykänen
Activity
ficristo commentedon Feb 13, 2017
Do you want to take a look at this?
tallandroid commentedon Feb 13, 2017
@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 commentedon Feb 14, 2017
@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 commentedon Feb 14, 2017
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 commentedon Feb 14, 2017
@ficristo Yep, I'd like to fix this issue, I'll take @humphd approach and once everything is done, I'll do a PR 👍
I'll have my PR later on this week :)
sdalmeida commentedon Feb 19, 2017
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.jsorProjectModel-test?Thanks :)
zaggino commentedon Feb 20, 2017
@Simon66
Where you put the tests depends mostly on where you fix the code. If you fix the code in
FileSystem.jsyou'd put the tests inFileSystem-test.js, when you fixProjectModel.jsyou'd put that intoProjectModel-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 intoFileTreeView-test.jssuite.sdalmeida commentedon Feb 24, 2017
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 commentedon Feb 24, 2017
@Simon66 thank you for the update!
Pomax commentedon Feb 24, 2017
@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 commentedon Apr 2, 2017
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 commentedon Apr 2, 2017
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.