search

openwebwork / webwork2

Course management front end for WeBWorK
http://webwork.maa.org/wiki/Main_Page
Other
140 stars 164 forks source link

characters when renaming files #2437

Open Alex-Jordan opened 3 weeks ago

Alex-Jordan commented 3 weeks ago

A strange thing happened in the course of testing #2430. I was trying to break the new functionality, and I uploaded course.conf. It wanted me to rename the file as expected, and I named it course\.conf to see what would happen. It uploaded the file as _conf. I don't really understand how it landed at that name, but maybe we need to restrict what characters can be used when renaming the file to upload. There is already a check like that when you are simply trying to rename a file.

drgrice1 commented 3 weeks ago

When a file is renamed the verifyPath method is called which insists that the given path consists only of the characters in -_.a-zA-Z0-9, and then it also verifies that path is okay (in that it doesn't try to leave the working directory, doesn't follow a symlink, and such), verifies that the file does not exist, and a few other things. That may work in this case also.

When a file is uploaded the checkName method is called on the file name. That method first removes all characters up to and including the last forward slash or backslash, then checks that all characters are valid (the same characters as those checked by the verifyPath method), and then changes a leading dot into an underscore. After all of that if nothing is left then it changes the file name to newfile.txt. Of course what is happening is that course\.conf is changed to .conf by the first step, all characters in that are okay so it proceeds to the third step, and that changes it to _conf.

I think the problem with the checkName method is that it tries to correct invalid characters and provide something that works. What really should be done the user should be told what is wrong with the filename, and then given a chance to fix it. Don't try to fix it for the user.