orhun / rustypaste

A minimal file upload/pastebin service.
https://blog.orhun.dev/blazingly-fast-file-sharing
MIT License
757 stars 47 forks source link

fix(server): return the correct file on multiple files with same name #234

Closed tessus closed 6 months ago

tessus commented 7 months ago

Description

The glob_match_file function returns the wrong entry. glob yields paths in alphabetical order. We want the file with the longest expiry date/time.

Motivation and Context

fixes #218

How Has This Been Tested?

Changelog Entry

### Changed

- Fixed an erroneous `file not found` occurrence

Types of Changes

Checklist:

codecov-commenter commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8de2450) 70.11% compared to head (5056916) 70.11%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #234 +/- ## ======================================= Coverage 70.11% 70.11% ======================================= Files 11 11 Lines 609 609 ======================================= Hits 427 427 Misses 182 182 ``` | [Flag](https://app.codecov.io/gh/orhun/rustypaste/pull/234/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) | Coverage Δ | | |---|---|---| | [unit-tests](https://app.codecov.io/gh/orhun/rustypaste/pull/234/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) | `70.11% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tessus commented 7 months ago

I thought about it for a while and it won't break anything.

But I see an issue that has nothing to do with this or the previous PRs: One can upload multiple files with the same name, but that was already possible with the curl -F "file=@x.txt;filename=wow.nice" command when random_url was not set. This case was never taken into consideration. We can look into this in a separate discussion or issue.

tessus commented 7 months ago

I created a separate discussion #241

Can you think of any edge cases that this would break backwards compatibility?

A bit more info:

If multiple files with the same name are on the server, the file with the longest expiry timestamp will be returned when accessing the file via curl <server>/file.txt

Previously a different file was returned. Which one is not entirely clear depending on expiration but not yet cleaned from disk, and so on.

But this was never a defined behavior anyway, thus no breaking change.