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

Renaming folders? #859

Closed
redherring917b opened this issue Sep 21, 2022 Discussed in #858 · 9 comments · Fixed by Langlais115/tinyfilemanager#1
Closed

Renaming folders? #859

redherring917b opened this issue Sep 21, 2022 Discussed in #858 · 9 comments · Fixed by Langlais115/tinyfilemanager#1

Comments

@redherring917b
Copy link

Discussed in #858

Originally posted by redherring917b September 20, 2022
Hi there.

I find myself unable to rename an existing folder. I had a similar issue with renaming files, until I realized that I had to add to my allowed list of $allowed_file_extensions in the config.php file that I'm using. But allowing extensions wouldn't apply to folders, obviously. Any thoughts on how to successfully rename a folder?

EDITED: To perhaps be clearer, when attempting to rename a folder I'm just getting the "Error while renaming from [oldname] to [newname]" message, so it would seem to be failing on this condition in tinyfilemanager.php:

if (fm_rename($path . '/' . $old, $path . '/' . $new)) {

I've output both the old and new paths and they are identical save for the changed folder name.

Thanks!

@redherring917b
Copy link
Author

Pretty sure the problem is that renaming a folder calls the function fm_is_valid_ext which doesn't find the fold name extension as being in the list of allowed extensions (since it's a folder and has no extension), and rejects the rename. I can change my $allowed_file_extensions settings to an empty list and then successfully rename a folder, but if I do that I can also then rename a file to one with an undesirable file extension. So it seems that the renaming of a folder needs to be identified and handled differently than the renaming of a file.

Anyone have suggestions / a fix for this?

Thanks!

@redherring917b
Copy link
Author

I sure thought this would work to exclude the renaming of folders from the check on allowed extensions for filenames, but sadly it does not.

function fm_rename($old, $new)
{	
	if(!is_dir($new)) {
    	        $isFileAllowed = fm_is_valid_ext($new);
    	        if(!$isFileAllowed) return false;
	} else {
		return (!file_exists($new) && file_exists($old)) ? rename($old, $new) : null;	
   	}    
}

Langlais115 added a commit to Langlais115/tinyfilemanager that referenced this issue Sep 25, 2022
Solve folder rename when file extension restriction is enabled.
Fixes prasathmani#859
Langlais115 added a commit to Langlais115/tinyfilemanager that referenced this issue Sep 25, 2022
Solve folder rename when file extension restriction is enabled.
Fixes prasathmani#859
@redherring917b
Copy link
Author

Cool! Thanks. Is there any chance you'd be able to pass on the specific changes that were required? I've made a number of minor revisions on this end that I don't want to lose in any "merge", so if the changes for this fix were minimal it might make me most comfortable to just implement those manually into my file.

Thanks again.

@Langlais115
Copy link

Just have a look in the commit.

But here it is anyway:

    if(!is_dir($old))
    {
        if (!$isFileAllowed) return false;
    }

@redherring917b
Copy link
Author

Ah yes, of course. Sorry about that! And thanks again. Very appreciative of the fix.

@Langlais115
Copy link

If you close an issue before the pull request is merged into the master branch, the pull request is canceled.
Therefore this issue is not fixed in the current code.

@redherring917b
Copy link
Author

Oh man. My bad. So sorry about that. Clearly I'm not familiar enough with the workings of Github. Reopening, and again, thank you.

@prasathmani
Copy link
Owner

This issue is addressed in the new release.

@chikuairi
Copy link

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants