Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Un-pinned stored files are not removed after the custom TTL is exceed #174

Open
Softvision-GeluHaiduc opened this issue Jan 21, 2021 · 4 comments
Labels
HC-January-Release Issues logged during the Hubs Cloud January Release jira-hubs-cloud [QA]:Normal-issue Label for QA to mark normal issues logged

Comments

@Softvision-GeluHaiduc
Copy link

Softvision-GeluHaiduc commented Jan 21, 2021

[Affected Versions]:

  • Firefox Release 84.0.2

[Affected Platforms]:

  • Windows 10x64

[Prerequisites]:

  • Make sure to be logged in as an admin and have the Admin Panel opened.

[Steps to reproduce]:

  1. Click the "Server Settings" list item from the left side menu in the "Setup" section.
  2. Click the "Content" tab on the page.
  3. Input a valid entry, 10, in the "Storage TTL" field and save the changes.
  4. Go create a room and upload a file to the room. (e.g. drag and drop an image into the room)
  5. Hover the object press the spacebar and click the "open link" button.
  6. Wait for the storage TTL time to expire and observe/reload the page of the object.

[Expected results]:

  • The website where the object is held fails to load.

[Actual results]:

  • The website where the object is held is still available and the object can be seen.

[Notes]:

  • The issue is reproducible with different values for the time.
  • The issue is also reproducible after closing the room.
  • Attached a screen recording of the issue here.

┆Issue is synchronized with this Jira Task

@Softvision-GeluHaiduc Softvision-GeluHaiduc added [QA]:Major-issue Label for QA to mark major issues logged HC-January-Release Issues logged during the Hubs Cloud January Release labels Jan 21, 2021
@johnshaughnessy
Copy link
Contributor

I do not think this needs to block the release because I think I gave you an incorrect test case for ttl.

Files are not removed immediately after their ttl expires. They are only "vacuum"ed once a day:

https://github.com/mozilla/reticulum/blob/42775acec0ac3de4e5c8b50e27610ce5a4e6e0ec/config/prod.exs#L122

Their ttl is checked during this vacuum job to determine whether or not they should be removed. Importantly, their ttl only expires if the file has not been accessed at all in the meantime:

https://github.com/mozilla/reticulum/blob/42775acec0ac3de4e5c8b50e27610ce5a4e6e0ec/lib/ret/storage.ex#L173-L186

Two remediation items we can proceed with:

  • The test case should instead be: Set the ttl to something short. Upload a file, save the link, close the room and don't access the link for 48 hours (24 hours may not be enough in case you happen to start just before the daily vacuum). After 48 hours pass, try to access the media at the link. It should not be found.
  • We may want to change the code to perform the ttl check whenever we fetch media from the @expiring_file_path bucket so that the ttl is enforced even on not-yet-vacuumed files:
    https://github.com/mozilla/reticulum/blob/42775acec0ac3de4e5c8b50e27610ce5a4e6e0ec/lib/ret/storage.ex#L64
    This will probably be more consistent with the hubs cloud operator's expectations.

@Softvision-GeluHaiduc
Copy link
Author

The test case should instead be: Set the ttl to something short. Upload a file, save the link, close the room and don't access the link for 48 hours (24 hours may not be enough in case you happen to start just before the daily vacuum). After 48 hours pass, try to access the media at the link. It should not be found.

I was in luck, I still had the bookmarks from when I tested it the first time, but more than 48 hours passed, the links from the objects gave an "HTTP ERROR 401".
Is this the correct behaviour/error code (401 Unauthorized)?

@johnshaughnessy
Copy link
Contributor

Is this the correct behaviour/error code (401 Unauthorized)?

I think that is correct.

I am not very familiar with this code and am surprised that this would not return a 404 not found. However it seems like 401 is the current intended behavior. Source:
https://github.com/mozilla/reticulum/blob/42775acec0ac3de4e5c8b50e27610ce5a4e6e0ec/lib/ret_web/controllers/file_controller.ex#L40-L41
and
https://github.com/mozilla/reticulum/blob/42775acec0ac3de4e5c8b50e27610ce5a4e6e0ec/lib/ret/storage.ex#L80-L81

@Softvision-GeluHaiduc
Copy link
Author

I will add the [QA]:Normal issue label as this is working with the updated test scenario.
I will leave it open as to tracking the possible changes mentioned in the first comment, if this is not necessary we can close it as well

@Softvision-GeluHaiduc Softvision-GeluHaiduc added [QA]:Normal-issue Label for QA to mark normal issues logged and removed [QA]:Major-issue Label for QA to mark major issues logged labels Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HC-January-Release Issues logged during the Hubs Cloud January Release jira-hubs-cloud [QA]:Normal-issue Label for QA to mark normal issues logged
Projects
None yet
Development

No branches or pull requests

2 participants