[BUG] Multiple files with the same name through invalid filenames

[togrue]

Describe the bug

When appending to a file with invalid characters in the file name, the file system may end up listing several files with the same name.

InvalidName: "log_1_14:03:21.txt" // ':' is an invalid character
compliantName: "log_1_14_03_21.txt" // Created by the function FF_MakeNameCompliant

The appending is done multiple times in this fashion:
Open the File "a+", write to it, and close it again.

What happens in the library:

1. FF_Open checks if the file exists using the invalidName
2. The file doesn't exist -> FF_CreateFile is called with the invalidName
3. FF_CreateDirent is called with the invalidName, which replaces the invalid characters silently
4. A entry with the compliantName is created.

It seems that the entry is created without checking for an existing entry in step (4).

The question is:
Is there a reason why invalid filenames should not lead to an error?

Target
Running on a Xilinx Zynq Ultrascale+ (Cortex A53)
Built on Windows with Vitis v2021.2.0

How to Reproduce
Attempt to append to a file with an invalid filename multiple times.
Repeated Sequence: Open file ("a+"), write to it, and close the file.

Expected behavior/solutions
Actually, i would expect an error indicating that the file name is invalid.

Logs

log_13576_2024-01-24_16_35_50.txt |           file |       2447 | 2024.01.24 Wed 16:35:50.000 |
log_13576_2024-01-24_16_35_50.txt |           file |       2447 | 2024.01.24 Wed 16:35:50.000 |
log_13576_2024-01-24_16_35_50.txt |           file |       2447 | 2024.01.24 Wed 16:35:50.000 |
log_13576_2024-01-24_16_35_50.txt |           file |       2447 | 2024.01.24 Wed 16:35:50.000 |

ff_findnext(..) returns the same name multiple times.
(Verified by the FreeRTOS ftp server implementation & our own implementation).

Additional context

Minimal example to reproduce the bug:

void createFile(const char* filename) {
    FF_FILE* file = ff_fopen(filename, "w");
    if (file != NULL) {
        ff_fclose(file);
    }
}

// creates two files, both are named 'inv_log.txt'
createFile("inv:log.txt");
createFile("inv:log.txt");

[Skptak]

Hey @togrue, thanks for bringing this issue to our attention!

I'll take a look into this when I can, in the meantime if you want to contribute a pull request to fix this issue that'd be great!

[Skptak]

I was wondering if there was any chance you could try wrapping the ff_dir.c:forbiddenChrs[] array in the same check used for the ffconfigUNICODE_UTF16_SUPPORT define?
I.e. going from this:

static const uint8_t forbiddenChrs[] =
{
/* Windows says: don't use these characters: '\/:*?"<>|'
 *  "     *     /	    :     <     >     ?    '\'    ?     | */
    0x22, 0x2A, 0x2F, 0x3A, 0x3C, 0x3E, 0x3F, 0x5C, 0x7F, 0x7C
};

To this:

#if ( ffconfigUNICODE_UTF16_SUPPORT != 0 )
    static const FF_T_WCHAR forbiddenChrs[] =
#else
    static const uint8_t forbiddenChrs[] =
#endif
{
/* Windows says: don't use these characters: '\/:*?"<>|'
 *  "     *     /	    :     <     >     ?    '\'    ?     | */
    0x22, 0x2A, 0x2F, 0x3A, 0x3C, 0x3E, 0x3F, 0x5C, 0x7F, 0x7C
};

and let me know if that helps with this issue?

These are "forbidden characters" for the filename, where not sure what the full level of support for bugs related to them is? But I'm not the most familiar with the inner workings of this library, @htibosch @cookpate what are your thoughts?

[togrue]

I'll send a pull-request this weekend.

Something like: a6cdf19

[htibosch]

Thanks a lot @togrue for reporting this. I am looking forward to you PR. If you need any support, please call.

It looks like the check for forbiddenChrs should happen earlier in the process.

[togrue]

How can the FreeRTOS-FAT library be tested without hw-access?

I could build the FreeRTOS Posix_GCC and WIN32-MinGW demo application, without the FreeRTOS-FAT library (on
my Win11 PC with WSL (Ubuntu) and MSYS2).

Is this project normally used?
https://www.freertos.org/FreeRTOS-Plus/FreeRTOS_Plus_TCP/examples_FreeRTOS_simulator.html#examples
However, for linux there seems to be this sddisk test-port (Lab-Project-FreeRTOS-FAT/portable/linux/ff_sddisk.c).

It looks like the check for forbiddenChrs should happen earlier in the process.

Do you have a specific place in mind?
FF_CreateDirent is called by three functions: FF_Move, FF_CreateFile, FF_MkDir (according to the call-hierarchy).

Note: It is not possible to change the FF_MakeNameCompliant to a checker only function,
because the function remaps the character 0xe5 to 0x05 (for storage).
Also, it seems that the replacement 0x05 is not done in the other direction from a dir-entry to a filename.
(So filenames starting with 0xE5 could also run into this duplicated file bug.
However, i couldn't test that because i have no test-setup at the moment.)

The previous sent commit hash: a6cdf19
(Tested for invalid file-names, but not dir-names)

[htibosch]

@togrue asked:

How can the FreeRTOS-FAT library be tested without hw-access?

I normally take a WinSim demo and add a RAM disk.

At this moment I'm still on a different project, after that I will look into the details of your report. Should not be difficult.

[Skptak]

@togrue, in the meantime don't hesitate to raise your commits that you're linking as a Pull Request to this repo, it makes it a bit easier to review the changes 😄

[AniruddhaKanhere]

Hello @togrue for creating the PR which I just approved and merged into mainline.

I shall be closing this issue. Please feel free to reopen this issue if you feel that your bug was not fixed.

Thanks again,
Aniruddha