Skip to content

Rename incorrectly cased files#30802

Closed
SharkyKZ wants to merge 9 commits intojoomla:stagingfrom
SharkyKZ:j3/fix/file-casing
Closed

Rename incorrectly cased files#30802
SharkyKZ wants to merge 9 commits intojoomla:stagingfrom
SharkyKZ:j3/fix/file-casing

Conversation

@SharkyKZ
Copy link
Contributor

@SharkyKZ SharkyKZ commented Sep 29, 2020

Closes #23725 and #30736 .

Summary of Changes

This PR attempts to implement case sensitive file rename and rename some incorrectly cased files.

Testing Instructions

Coming soon.

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

No.

@HLeithner
Copy link
Member

I would prefer to have this in the framework at least the actual renaming including tests for all possible variants.

@HLeithner
Copy link
Member

@SharkyKZ I merged current j3 into your pr and tested the upgrade and some basic functionality and it still works. Would you remove the draft status. @bembelimen cherry picked the relevant commit for j4 in #31804 to fix the release blocker.

So it would makes sense to merge this into j3 to complete the task. thanks

@HLeithner HLeithner added this to the Joomla! 3.9.24 milestone Dec 30, 2020
@bembelimen
Copy link
Contributor

bembelimen commented Jan 5, 2021

Test cases:
Install Joomla! <= 3.9.23 => check the following files: https://github.com/joomla/joomla-cms/pull/30802/files#diff-db7eb77540ff419bd7e6557d2779f4cf1e31158c20cb4b63dbbe8d6c426385adR2670-R2674
Use https://ci.joomla.org/artifacts/joomla/joomla-cms/staging/30802/downloads/38795/ for Update

  • Update with the Joomla Updater
  • Copy the files via FTP and use "Fix Database"
  • Install the package as a fresh installation

Now check the file list again => files should in all three test cases have the correct name.

Especially on windows system tests are needed.

@bembelimen
Copy link
Contributor

I have tested this item ✅ successfully on f349349


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30802.

@wilsonge
Copy link
Contributor

wilsonge commented Jan 6, 2021

Following @bembelimen testing instructions above on OSX all the files mention are deleted after applying patch and not renamed. Admittedly OSX isn't exactly a realistic host. But yeah not working :(

@chmst
Copy link
Contributor

chmst commented Jan 6, 2021

Tested on win10 and it works as described, Files are renamed.

@softforge
Copy link
Contributor

I have tested this item ✅ successfully on f349349

I have tested this on a centos server (linux) and it worked like a charm. All the test cases were correct after the change


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30802.

@richard67
Copy link
Member

@wilsonge Do you think you can find the time to add some debug log (error_log or whatever) to see what goes wrong on OSX?

@HLeithner
Copy link
Member

@wilsonge can you run only the relevant code?

@HLeithner
Copy link
Member

I postponed it after 3.9.24 release so not urgent except that j4 waits for this pr #31804 and PRs pending on it like skipTo js update

@wilsonge
Copy link
Contributor

wilsonge commented Jan 7, 2021

<?php

function fixFilenameCasing()
{
    $files = array(
        'libraries/src/Filesystem/Support/Stringcontroller.php' => 'libraries/src/Filesystem/Support/StringController.php',
//        'libraries/vendor/paragonie/sodium_compat/src/Core/Xsalsa20.php' => 'libraries/vendor/paragonie/sodium_compat/src/Core/XSalsa20.php',
//        'media/mod_languages/images/si_LK.gif' => 'media/mod_languages/images/si_lk.gif',
    );

    foreach ($files as $old => $expected)
    {
        $oldRealpath = realpath(__DIR__ . '/' . $old);

        // On Unix without incorrectly cased file.
        if ($oldRealpath === false)
        {
            continue;
        }

        $oldBasename      = basename($oldRealpath);
        $newRealpath      = realpath(__DIR__ . '/' . $expected);
        $newBasename      = basename($newRealpath);
        $expectedBasename = basename($expected);

	    var_dump('Old Real Path: ' . $oldRealpath);
	    var_dump('New Real Path: ' . $newRealpath);
	    var_dump('New Base Name: ' . $newBasename);
	    var_dump('Expected Base Name: ' . $expectedBasename);

	    // On Windows with incorrectly cased file.
        if ($newBasename !== $expectedBasename)
        {
            // Rename the file.
            rename(__DIR__ . '/' . $old, __DIR__ . '/' . $old . '.tmp');
            rename(__DIR__ . '/' . $old . '.tmp', __DIR__ . '/' . $expected);

            continue;
        }

        // On Unix with correctly and incorrectly cased files.
        if ($oldBasename === basename($old) && $newBasename === $expectedBasename)
        {
            // Delete the file.
            unlink(__DIR__ . '/' . $old);
        }
    }
}

fixFilenameCasing();

So I've run this in the root dir of Joomla to decouple of it - same behaviour. var_dumps on OSX give me:

string(99) "Old Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/Stringcontroller.php"
string(99) "New Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/StringController.php"
string(35) "New Base Name: StringController.php"
string(40) "Expected Base Name: StringController.php"

Not 100% sure what I'm expecting to be seeing on other filesystems as a comparison though. If you and @bembelimen could give the results of those 4 vars on windows and linux would be helpful so I can compare

Screenshot 2021-01-07 at 01 54 05

And the path libraries/src/Filesystem/Support is empty of any files so it's not a git status bug ;)

@richard67
Copy link
Member

richard67 commented Jan 7, 2021

@wilsonge The problem might be that the stringcontroller.php file is still in the list of files to be removed:

'/libraries/joomla/filesystem/support/stringcontroller.php',
. I think the files to be renamed should not be in the list of files to be removed. For the other files to be renamed, the language flag and the xsalsa thing, this is given.

Update after 2 minutes: Sorry, I was wrong, the path to the file is different, so that should not be the problem.

@richard67
Copy link
Member

richard67 commented Jan 7, 2021

@wilsonge Could you also log the value of $newRealpath? Sorry, I need more coffee.

@richard67
Copy link
Member

richard67 commented Jan 7, 2021

@wilsonge I think the problem is what is mentioned in the comment on https://www.php.net/manual/en/function.realpath.php:

Note:
For case-insensitive filesystems realpath() may or may not normalize the character case.

That seems to work differently on Windows and OSX: On Windows the $newRealpath could be false because the file doesn't exist, while on OSX it is as shown in your test.

@bembelimen Could you check the value of $newRealpathon Windows?

Update: See also discussion in https://bugs.php.net/bug.php?id=67220.

@richard67
Copy link
Member

richard67 commented Jan 7, 2021

@wilsonge I think the solution could be to use pathinfo(..., PATHINFO_BASENAME); instead of basename(...) (except of the $expectedBasename). As https://www.php.net/manual/en/function.basename.php says, the basename function just uses the input string and not the real file info.

That would be for your testing script:

<?php

function fixFilenameCasing()
{
    $files = array(
        'libraries/src/Filesystem/Support/Stringcontroller.php' => 'libraries/src/Filesystem/Support/StringController.php',
//        'libraries/vendor/paragonie/sodium_compat/src/Core/Xsalsa20.php' => 'libraries/vendor/paragonie/sodium_compat/src/Core/XSalsa20.php',
//        'media/mod_languages/images/si_LK.gif' => 'media/mod_languages/images/si_lk.gif',
    );

    foreach ($files as $old => $expected)
    {
        $oldRealpath = realpath(__DIR__ . '/' . $old);

        // On Unix without incorrectly cased file.
        if ($oldRealpath === false)
        {
            continue;
        }

        $oldBasename      = pathinfo($oldRealpath, PATHINFO_BASENAME);
        $newRealpath      = realpath(__DIR__ . '/' . $expected);
        $newBasename      = pathinfo($newRealpath, PATHINFO_BASENAME);
        $expectedBasename = basename($expected);

        var_dump('Old Real Path: ' . $oldRealpath);
        var_dump('New Real Path: ' . $newRealpath);
        var_dump('New Base Name: ' . $newBasename);
        var_dump('Expected Base Name: ' . $expectedBasename);

        // On Windows with incorrectly cased file.
        if ($newBasename !== $expectedBasename)
        {
            // Rename the file.
            rename(__DIR__ . '/' . $old, __DIR__ . '/' . $old . '.tmp');
            rename(__DIR__ . '/' . $old . '.tmp', __DIR__ . '/' . $expected);

            continue;
        }

        // On Unix with correctly and incorrectly cased files.
        if ($oldBasename === basename($old) && $newBasename === $expectedBasename)
        {
            // Delete the file.
            unlink(__DIR__ . '/' . $old);
        }
    }
}

fixFilenameCasing();

@richard67
Copy link
Member

Meanwhile it turned out that my above suggestion won't help, thanks @wilsonge for testing on OSX. Am investigating possible solutions. Stay tuned.

@HLeithner
Copy link
Member

it may or may not be osx it self, it could depend on the filesystem used in osx and if it's setting os x supports both case sensitive and insensitive files per setting per filesystem.

btw. I think it's possible we have the same problem on windows and linux.

@richard67
Copy link
Member

it may or may not be osx it self, it could depend on the filesystem used in osx and if it's setting os x supports both case sensitive and insensitive files per setting per filesystem.

I agree. But I think I have a solution almost ready.

@richard67
Copy link
Member

@wilsonge or whoever else has OSX for testing: Please test the following test script:

<?php

function fixFilenameCasing()
{
    $files = array(
        'libraries/src/Filesystem/Support/Stringcontroller.php' => 'libraries/src/Filesystem/Support/StringController.php',
//        'libraries/vendor/paragonie/sodium_compat/src/Core/Xsalsa20.php' => 'libraries/vendor/paragonie/sodium_compat/src/Core/XSalsa20.php',
//        'media/mod_languages/images/si_LK.gif' => 'media/mod_languages/images/si_lk.gif',
    );

    foreach ($files as $old => $expected)
    {
        $oldRealpath = realpath(__DIR__ . '/' . $old);

        // On case-sensitive file system without incorrectly cased file.
        if ($oldRealpath === false)
        {
            continue;
        }

        $oldBasename      = basename($oldRealpath);
        $oldInode         = fileinode($oldRealpath);
        $newRealpath      = realpath(__DIR__ . '/' . $expected);
        $newBasename      = basename($newRealpath);
        $newInode         = fileinode($newRealpath);
        $expectedBasename = basename($expected);

        var_dump('Old Real Path: ' . $oldRealpath);
        var_dump('New Real Path: ' . $newRealpath);
        var_dump('Old Base Name: ' . $oldBasename);
        var_dump('basename($old): ' . basename($old));
        var_dump('New Base Name: ' . $newBasename);
        var_dump('Expected Base Name: ' . $expectedBasename);
        var_dump('Old Inode: ' . $oldInode);
        var_dump('New Inode: ' . $newInode);

        // On case-insensitive file system with incorrectly cased file or where it's not clear.
        if ($newBasename !== $expectedBasename || $newInode === $oldInode)
        {
            // Rename the file.
            rename(__DIR__ . '/' . $old, __DIR__ . '/' . $old . '.tmp');
            rename(__DIR__ . '/' . $old . '.tmp', __DIR__ . '/' . $expected);

            continue;
        }

        // On case-sensitive file system with correctly and incorrectly cased files.
        if ($oldBasename === basename($old) && $newBasename === $expectedBasename)
        {
            // Delete the file.
            unlink(__DIR__ . '/' . $old);
        }
    }
}

fixFilenameCasing();

@wilsonge
Copy link
Contributor

wilsonge commented Jan 8, 2021

That works. But it does try and rename the file on every run.

string(99) "Old Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/Stringcontroller.php"
string(99) "New Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/StringController.php"
string(35) "Old Base Name: Stringcontroller.php"
string(36) "basename($old): Stringcontroller.php"
string(35) "New Base Name: StringController.php"
string(40) "Expected Base Name: StringController.php"
string(21) "Old Inode: 8689842387"
string(21) "New Inode: 8689842387"

@richard67
Copy link
Member

richard67 commented Jan 8, 2021

That works. But it does try and rename the file on every run.

I know 😄 . No idea how I can avoid that.

P.S.: That's only a problem on that kind of file system. On Linux it tries it only 1 time.

P.P.S.: ... and it is a problem on Windows now, too, I think.

@richard67
Copy link
Member

@wilsonge Do you also have Windows for testing?

@richard67
Copy link
Member

richard67 commented Jan 8, 2021

@wilsonge Could you add following to the test script just below the other var_dump and report back the result?

var_dump('glob($oldRealpath, GLOB_NOESCAPE)[0]: ' . glob($oldRealpath, GLOB_NOESCAPE)[0]);
var_dump('glob($newRealpath, GLOB_NOESCAPE)[0]: ' . glob($newRealpath, GLOB_NOESCAPE)[0]);

Update: Meanwhile I can test on Windows, too. Using e.g. glob($oldRealpath, GLOB_NOESCAPE)[0] would not change anything for Linux and Windows, i.e. things still would work. So the answer to this question here would tell us if it would help for OSX.

@richard67
Copy link
Member

richard67 commented Jan 8, 2021

@wilsonge New testing script:

<?php

function fixFilenameCasing()
{
    $files = array(
        'libraries/src/Filesystem/Support/Stringcontroller.php' => 'libraries/src/Filesystem/Support/StringController.php',
//        'libraries/vendor/paragonie/sodium_compat/src/Core/Xsalsa20.php' => 'libraries/vendor/paragonie/sodium_compat/src/Core/XSalsa20.php',
//        'media/mod_languages/images/si_LK.gif' => 'media/mod_languages/images/si_lk.gif',
    );

    foreach ($files as $old => $expected)
    {
        $oldRealpath = glob(realpath(__DIR__ . '/' . $old), GLOB_NOESCAPE)[0] ?? false;

        // On case-sensitive file system without incorrectly cased file.
        if ($oldRealpath === false)
        {
            continue;
        }

        $oldBasename      = basename($oldRealpath);
        $newRealpath      = glob(realpath(__DIR__ . '/' . $expected), GLOB_NOESCAPE)[0] ?? false;
        $newBasename      = basename($newRealpath);
        $expectedBasename = basename($expected);

        var_dump('Old Real Path: ' . $oldRealpath);
        var_dump('Old Base Name: ' . $oldBasename);
        var_dump('basename($old): ' . basename($old));
        var_dump('New Real Path: ' . $newRealpath);
        var_dump('New Base Name: ' . $newBasename);
        var_dump('Expected Base Name: ' . $expectedBasename);

        // On any file system with incorrectly cased file only.
        if ($newBasename !== $expectedBasename)
        {
            // Rename the file.
            rename(__DIR__ . '/' . $old, __DIR__ . '/' . $old . '.tmp');
            rename(__DIR__ . '/' . $old . '.tmp', __DIR__ . '/' . $expected);

            continue;
        }

        // On case-sensitive file system with correctly and incorrectly cased files.
        if ($oldBasename === basename($old))
        {
            // Delete the file.
            unlink(__DIR__ . '/' . $old);
        }
    }
}

fixFilenameCasing();

@wilsonge
Copy link
Contributor

wilsonge commented Jan 9, 2021

george@ ~/Sites/joomla-cms (fix/totp-feature) $ php test.php 
string(99) "Old Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/Stringcontroller.php"
string(35) "Old Base Name: Stringcontroller.php"
string(36) "basename($old): Stringcontroller.php"
string(99) "New Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/StringController.php"
string(35) "New Base Name: StringController.php"
string(40) "Expected Base Name: StringController.php"

it just deletes the file

@richard67
Copy link
Member

So no change. Then we either do my previous idea with the inode which renames the file always, or I have no idea what else we can do.

@wilsonge
Copy link
Contributor

wilsonge commented Jan 9, 2021

The extra var_dumps are more interesting though -

string(122) "glob($oldRealpath, GLOB_NOESCAPE)[0]: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/Stringcontroller.php"
string(122) "glob($newRealpath, GLOB_NOESCAPE)[0]: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/StringController.php"

@richard67
Copy link
Member

Interesting .. and exactly that's the problem. We don't really get the actual case.

@richard67
Copy link
Member

richard67 commented Jan 10, 2021

@wilsonge Next iteration: Could you test with the following test script?

<?php

function fixFilenameCasing()
{
    $files = array(
        'libraries/src/Filesystem/Support/Stringcontroller.php' => 'libraries/src/Filesystem/Support/StringController.php',
//        'libraries/vendor/paragonie/sodium_compat/src/Core/Xsalsa20.php' => 'libraries/vendor/paragonie/sodium_compat/src/Core/XSalsa20.php',
//        'media/mod_languages/images/si_LK.gif' => 'media/mod_languages/images/si_lk.gif',
    );

    foreach ($files as $old => $expected)
    {
        $oldRealpath = realpath(__DIR__ . '/' . $old);

        // On Unix without incorrectly cased file.
        if ($oldRealpath === false)
        {
            echo "Unix without incorrectly cased file.\n";

            continue;
        }

        $oldBasename      = basename($oldRealpath);
        $newRealpath      = realpath(__DIR__ . '/' . $expected);
        $newBasename      = basename($newRealpath);
        $expectedBasename = basename($expected);

        var_dump('Old Real Path: ' . $oldRealpath);
        var_dump('Old Base Name: ' . $oldBasename);
        var_dump('basename($old): ' . basename($old));
        var_dump('New Real Path: ' . $newRealpath);
        var_dump('New Base Name: ' . $newBasename);
        var_dump('Expected Base Name: ' . $expectedBasename);

        // On Windows or Unix with only the incorrectly cased file.
        if ($newBasename !== $expectedBasename)
        {
            echo "Renaming the file.\n";

            // Rename the file.
            rename(__DIR__ . '/' . $old, __DIR__ . '/' . $old . '.tmp');
            rename(__DIR__ . '/' . $old . '.tmp', __DIR__ . '/' . $expected);

            continue;
        }

        // There might still be an incorrectly cased file on other OS than Windows.
        if ($oldBasename === basename($old))
        {
            // Check if case-insensitive file system, eg on OSX.
            if (fileinode($oldRealpath) === fileinode($newRealpath))
            {
                // Check deeper because even realpath or glob might not return the actual case.
                if (!in_array($expectedBasename, scandir(dirname($newRealpath))))
                {
                    echo "Renaming the file on OSX.\n";

                    // Rename the file.
                    rename(__DIR__ . '/' . $old, __DIR__ . '/' . $old . '.tmp');
                    rename(__DIR__ . '/' . $old . '.tmp', __DIR__ . '/' . $expected);
                }
            }
            else
            {
                // On Unix with correctly and incorrectly cased files.
                echo "Deleting the incorrectly cased file on Unix.\n";

                unlink(__DIR__ . '/' . $old);
            }
        }
    }
}

fixFilenameCasing();

Thanks in advance.

@wilsonge
Copy link
Contributor

wilsonge commented Jan 10, 2021

george@ ~/Sites/joomla-cms (4.0-dev) $ php test.php 
string(99) "Old Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/Stringcontroller.php"
string(35) "Old Base Name: Stringcontroller.php"
string(36) "basename($old): Stringcontroller.php"
string(99) "New Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/StringController.php"
string(35) "New Base Name: StringController.php"
string(40) "Expected Base Name: StringController.php"
Renaming the file on OSX.
george@ ~/Sites/joomla-cms (4.0-dev) $ php test.php 
string(99) "Old Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/Stringcontroller.php"
string(35) "Old Base Name: Stringcontroller.php"
string(36) "basename($old): Stringcontroller.php"
string(99) "New Real Path: /Users/george/Sites/joomla-cms/libraries/src/Filesystem/Support/StringController.php"
string(35) "New Base Name: StringController.php"
string(40) "Expected Base Name: StringController.php"

I'd say that looks pretty good!

@richard67
Copy link
Member

@richard67
Copy link
Member

As @SharkyKZ doesn't want to merge my PR I have made a new PR which replaces this one here. Please test #32006 .

@richard67
Copy link
Member

Closing in favour of #32006 .

@richard67 richard67 closed this Jan 11, 2021
@richard67
Copy link
Member

@bembelimen @chmst @softforge @wilsonge Please test PR #32006 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stub generator script

8 participants