Skip to content

Conversation

@hamza221
Copy link
Contributor

@hamza221 hamza221 commented May 22, 2025

Summary

Make sure that files exist before downloading

reproduction

Scenario 1:

  1. Share a file with user1
  2. login as user 1
  3. delete share
  4. don't refresh page as user1
  5. download file
  6. opening/downloading the "missing" shared file causes "Page not found" errors

Scenario 2:

  1. create a federated share
  2. receiver can open the share
  3. federated share is removed by owner
  4. receiver does not refresh the folder and so does not recognize the removal
  5. opening/downloading the "missing" shared file causes "Page not found" errors

Checklist

Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
@hamza221 hamza221 self-assigned this May 22, 2025
@hamza221 hamza221 requested review from a team and skjnldsv as code owners May 22, 2025 13:00
@hamza221 hamza221 added the bug label May 22, 2025
@hamza221 hamza221 requested review from artonge and susnux and removed request for a team May 22, 2025 13:00
@hamza221 hamza221 added 3. to review Waiting for reviews feature: files labels May 22, 2025
@susnux
Copy link
Contributor

susnux commented May 22, 2025

What is fixed by this? For me it seems like a lot of unnecessary requests to the server.
Imagine to download a full server, you would do like X expensive PROPFIND just to the do a GET request.

Instead the GET should just fail with 404 which is an expected behavior.
IMHO a proper fix would be to have to kind of notification of changes causing the webui to reload, just like mobile clients do.

Meaning I do not see a benefit here expect from spamming the server with unneeded requests.

@susnux
Copy link
Contributor

susnux commented May 22, 2025

(federated share) is removed by owner
receiver does not refresh the folder and so does not recognize the removal

This can also happen between your PROPFIND and the real download, this does not ensure that it exists. Its not locked or similar.

@hamza221
Copy link
Contributor Author

hamza221 commented May 26, 2025

As mentioned above this would cause performance issues and still doesn't solve all edge cases. an adequate solution would be implementing #2742

@hamza221 hamza221 closed this May 26, 2025
@hamza221 hamza221 deleted the fix/noid/check-file-before-download branch May 26, 2025 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants