orhun
feat(server): overriding filename when paste from remote URL
Description
Add support for the filename: abc.txt header when uploading a paste from remote URL. So for example, this will now work:
curl -F "remote=https://example.com/file.png" -H "filename: foobar.png" "<server_address>"
Motivation and Context
Previously, the filename: header only worked for regular pastes, when the data is included in request's multipart/form-data. This addition makes the set of supported headers work more uniformly across other orthogonal features (remote vs. direct upload). Also, I personally needed this feature for reuploading files on discord's CDN, for an IRC bridge.
How Has This Been Tested?
- Unit test for changes in
Paste::store_remote_file - Unit test for server
test_upload_remote_file_override_filename() - Test fixture
test_upload_remote_file_override_filename - Testing on my own instance deployment, with curl (a command like the example above)
Changelog Entry
Added
- Add support for overriding the filename, using the
filename: abc.txtheader, when uploading a paste from remote URL.
Types of Changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation (no code change)
- [ ] Refactor (refactoring production code)
- [ ] Other
Checklist:
- [x] My code follows the code style of this project.
- [x] I have updated the documentation accordingly.
- [x] I have formatted the code with rustfmt.
- [x] I checked the lints with clippy.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
Also, I personally needed this feature for reuploading files on discord's CDN, for an IRC bridge.
If you reupload a file with the same name, it will be rejected with an error, unless you deleted the other file first. But I think I get what you are saying.
The only issue I see is that this repo requires signed commits.
@orhun do you require signed commits for all your projects/repos or was this by mistake? In theory it makes sense, but what if a dev does not have gpg set up? Do commits signed with an ssh key count as well? Also, if you squash and merge, the commit will have your signature anyway. IMO such a requirement only makes sense for a project, if multiple people push commits directly without creating PRs. (But that's just my opinion.)
If you reupload a file with the same name, it will be rejected with an error, unless you deleted the other file first. But I think I get what you are saying.
Yes, to be exact my use case is Discord CDN ur looks like https://cdn.discordapp.com/xxxx/yyyy/filename.jpg, and I want to reupload yyyy-filename.jpg as the paste filename (yyyy here is a unique id for the message that included this file).
Also, the use of fs::create_dir in a few tests like this one causes that, if any at point the these tests failed, subsequent ones will also fail with
---- server::tests::test_list stdout ----
Error: Os { code: 17, kind: AlreadyExists, message: "File exists" }
which confused for a while. I thought this was failing somewhere else, since there is no stacktrace, since it's not from an assert.
I don't know if you want to replace it with fs::create_dir_all (I believe it works fine if the directory already exists), or try to delete the dir first in each test, or just document this somewhere. Just FYI :)
Uploading a file with the same name does not overwrite the old file. An error in such a case is on purpose. Allowing to overwrite files can be dangerous since we don't have a user based access control. If you and I had tokens to upload files, we could replace each other's files (by mistake). That's why we decided to throw an error. If we both had delete tokens, we could yet again delete each other's files, but at least we'd know that something was off and the file wouldn't be automatically replaced.
I don't know if you want to replace it with fs::create_dir_all (I believe it works fine if the directory already exists), or try to delete the dir first in each test, or just document this somewhere. Just FYI :)
Thanks. Yea, I ran into this a few times myself. The tests have to start cleanly. If a test fails, the upload dir is not removed and it will cause a problem the next time the tests are run.
I never thought about fixing it, since I just use a script to remove left over dirs before starting the tests. We can ask orhun, if he wants a fix.
Let's see what Orhun says about the signed commits.
Ok, I'm heading to bed. ;-)
@orhun, we can't merge b/c of the signed commit requirement. I asked earlier whether this requirement was necessary, because of https://github.com/orhun/rustypaste/pull/504#issuecomment-3621262717