Fix to prevent moving files by renaming files using FileTreeView

[tallandroid]

1. Added / to be an invalid file name character for mac platform
2. Return baseName to include ../ if the fullpath contains ../

[petetnt]

../ is an valid expression on Windows too. So the function should be re-factored as:

    function getBaseName(fullPath) {
        var lastSlash = fullPath.lastIndexOf("/");
        var preFinalSlash = fullPath.substring(0, lastSlash).lastIndexOf("/");

        if (lastSlash === fullPath.length - 1) {  // directory: exclude trailing "/" too
            return fullPath.slice(fullPath.lastIndexOf("/", fullPath.length - 2) + 1, -1);
        } else if (fullPath.substring(preFinalSlash, lastSlash + 1) === "/../") {
            return fullPath.slice(preFinalSlash + 1);
        }
        return fullPath.slice(lastSlash + 1);
    }

However, it's only a partial fix for all the issues listed in #11609. It solves the main one, but the others still remain.

Some tests cases that should fail:

../helloworld.js
hello/world.js
/helloworld.js
helloworld.js/
../hello/../world.js

[tallandroid]

The fix covers the cases like :
../helloworld.js
../hello/../world.js

hello/world.js and helloworld.js/ are already covered by appshell rename validation logic. /helloworld.js is already covered.

[petetnt]

@tallandroid yeah, this PR does fix the original issue that #11609 so maybe the other things I mentioned are out of the scope for this PR (albeit related to my original issue #11609 (comment)).

Which platform are you on? At least on Windows the validation fails with /:s (these are tested against this patch). Check something like this:

➡️ folder foo doesn't exist, it throws an error

Let's create a folder

and try that again

it actually created the other file too (like one could except if relative paths would be valid filename:s

(The foo/bar.js file opens the correct file too, so there's that).

Similar things can be done with things like creating folders by naming a file with ending slash et cetera and it can be abused to infinity. That's why I think it's important that the fix would apply to all the cases regarding /'s in filenames.

[tallandroid]

I get your point. I think it makes more sense to weed out validation against base name completely. That is the safest path to go. I have updated the PR. I would appreciate if you can validate it on windows platform.

I verified the use cases on mac platform.

[petetnt]

Works great for renaming old files on Windows 👍

However, creating a new file and renaming that instantly skips _renameItem all together so at that part exploiting /'s is still possible:

Check out this if-clause

brackets/src/project/ProjectModel.js

Line 959 in d939740

if (renameInfo.type === FILE_CREATING) {

and the method createAtPath that it uses to create the file:

brackets/src/project/ProjectModel.js

Line 992 in d939740

ProjectModel.prototype.createAtPath = function (path) {

[tallandroid]

I intentionally left out createAtPath workflow as I believe its out of the scope of this PR. I completely understand the workflow mentioned above but I am skeptical of this being called a bug or not. Couple of reasons :

1. The file gets created at the path intended and gets shown correctly in Working Files section.
2. FileTreeView shows it as a separate file, but on refresh it gets pushed to the right path, so I am assuming that is what is expected. What is required is refreshing the FileTreeView on create like the way we do it for rename.

I would prefer discussing that over the group once and once confirmed , I myself am willing to make that change too :)

[petetnt]

Yeah, I am okay with this PR focusing on the renaming files part too 👍

ping @peterflynn @abose @nethip @humphd

[tallandroid]

Ping. I am not sure abt the turnaround time.

[abose]

Sorry for the late response .
@tallandroid @petetnt What is left to be done in this PR?

[petetnt]

@abose I think it's ready to be merged, the other related issues can be (and should be) fixed and tracked separately.

[tallandroid]

@abose is this done ? Let me know if you are waiting on something.

[abose]

Thanks @tallandroid .
Will merge once 1.6 is branched out to release.

[tallandroid]

Ping ?

[zaggino]

Reviewed and merged (as it should have been a while ago).

[petetnt]

Thanks @zaggino and especially @tallandroid for your patience!