Open Softvision-GeluHaiduc opened 3 years ago
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:
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:
Two remediation items we can proceed with:
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.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. 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 dailyvacuum
). 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)?
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
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
[Affected Versions]:
[Affected Platforms]:
[Prerequisites]:
[Steps to reproduce]:
[Expected results]:
[Actual results]:
[Notes]:
┆Issue is synchronized with this Jira Task