Skip to content

Implement STOU (Store Unique) command#91

Closed
ashhermirza wants to merge 3 commits into
eclipse-ecal:masterfrom
ashhermirza:STOU_CMD
Closed

Implement STOU (Store Unique) command#91
ashhermirza wants to merge 3 commits into
eclipse-ecal:masterfrom
ashhermirza:STOU_CMD

Conversation

@ashhermirza
Copy link
Copy Markdown
Contributor

@ashhermirza ashhermirza commented Dec 8, 2025

Resolves: [#90]

This PR introduces full support for the STOU (Store Unique) command, allowing FTP clients to upload files to the server while instructing the server to generate a unique, non-colliding filename.

Implementation Details

Unique Name Generation Logic

The server now generates a unique filename based on the current time and a random number to guarantee uniqueness.
Format: The generated filename adheres to the pattern: YYYYMMDDHHMMSS_RRRR_FILE.
YYYYMMDDHHMMSS: Current timestamp in Coordinated Universal Time (UTC).
RRRR: A 4-digit zero-padded random number.
Example Name: 20251207153045_0423_FILE

Pathing: The unique file is created within the user's current FTP Working Directory (ftp_working_directory_).

Pre-Transfer Checks

The command execution includes necessary validation checks before proceeding with the data transfer:
Verifies that the user is logged in.
Checks for the required FileWrite permission.
Ensures a valid data connection (data_acceptor_) has been established (via prior PORT or PASV command).

Compliance and Response

RFC Compliance: The server is responsible for returning the unique name to the client. The absolute file path, which contains the unique name, is included in the 150 intermediate response message: 150 Opening data connection. FILE: [absolute_file_path]

Unique Check: The implementation includes a check to verify that the generated unique file path does not already exist on the local file system using Filesystem::FileStatus. If a file name collision is detected after generation (highly unlikely), the action is aborted with 500.

Cross-Platform Time Handling

The code includes conditional compilation blocks (#if defined(unix) || defined(APPLE), #elif defined(_MSC_VER), etc.) to safely use thread-safe versions of time functions (gmtime_r or gmtime_s) for generating the timestamp component.

Testing

STOU_FTP_TEST.txt

@FlorianReimold FlorianReimold self-requested a review December 11, 2025 07:06
@FlorianReimold
Copy link
Copy Markdown
Member

FlorianReimold commented Dec 11, 2025

Thank you very much for this PR. Have you verified the code to actually work for non "/" FTP root paths? Because I reviewed the code and think that it returns the local file path instead of the virtual FTP file path. This not only leaks the server-side directory structure, but also renders the file inaccessible later on, because the client cannot find it under the returned path. You probably tested with an FTP Server on "/", so your local path and the virtual FTP path were equivalent.

Note: toLocalPath internally checks wether the given path is absolute or relative and adds the ftp_working_dir_ as needed. You must not prepend it beforehand, as that disturbs the checks. The returned path then points to the local filesystem and therefore must not be returned to the client.

I also think that fineftp should return the relative path (i.e. the filename only) instead of the absolute path. At least that is what vsftpd does. I couldn't find anything in the RFCs that mentions that specifically, but it usually is a good idea in these cases to behave equivalent to other major FTP servers.

@ashhermirza
Copy link
Copy Markdown
Contributor Author

@FlorianReimold, Thank you for catching that! You are absolutely right that returning the local file path was a major issue.

My initial testing on localhost hid the problem of leaking the server's directory structure and causing problems for the client.

I have updated the code to return only the unique file name (the relative path) in the response message, which is what is expected and aligns with other FTP servers like vsftpd.

I have re-tested the changes over a real network setup (server on one IP, client on another), and the file transfer works correctly. I also verified that the uploaded file is readable and intact on the server once the transfer is complete.

@FlorianReimold
Copy link
Copy Markdown
Member

Hi @ashhermirza ,

I found some time to take a look at the PR. I think the returned filename is fine now. However, I am certain that the randomness you are using here is very likely to cause issues for STOU commands that occur in the same second, as all of those commands will lead to the same filename. The issue is caused by re-seeding rand with a timestamp in second-precision.

Please check out the following godbolt link that simulates the Random-number-generation of 2 STOU commands:

https://godbolt.org/z/79rW43Tbx

A better approach to generate a random number would be:

std::random_device rd;
std::mt19937 gen(rd());
std::uniform_int_distribution<int> dist(0, 9999);
auto random_number = dist(gen);

The collision probability can be further reduced by creating bigger numbers, but at least this can create different numbers even when being called in the same second:

https://godbolt.org/z/jh4canxqq

Obviously, using a random number generator will never actually guarantee a unique filename though.

@ashhermirza
Copy link
Copy Markdown
Contributor Author

@FlorianReimold
Thanks for catching that - good point. I had initially added that logic for testing and overlooked updating the random number generation accordingly. I've now addressed it as per your suggestion.
Please let me know if you see anything else that needs to be adjusted.

@FlorianReimold
Copy link
Copy Markdown
Member

I re-implemented the command with better entropy and tests in #92 . Thus I am closing this.

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 this pull request may close these issues.

2 participants