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

viur-blob-locks entities are just using id_or_name #1214

Open
sveneberth opened this issue Jul 12, 2024 · 0 comments
Open

viur-blob-locks entities are just using id_or_name #1214

sveneberth opened this issue Jul 12, 2024 · 0 comments
Assignees
Labels
bug(fix) Something isn't working or address a specific issue or vulnerability Priority: Critical This should be dealt with ASAP. It's blocking someone.

Comments

@sveneberth
Copy link
Member

sveneberth commented Jul 12, 2024

I need to test and recheck this, but as far as I see there is a problem with viur-blob-locks.

viur-blob-locks prevents files (whether weak or not) from being deleted, even if the FileSkel has already been deleted.

When saving the skeleton, all the dlkeys used are collected with the help of getReferencedBlobs and saved in a viur-block-locks entity. However, this only uses the id_or_name of the skeleton key.

if not is_add and (old_blob_lock_obj := db.Get(db.Key("viur-blob-locks", db_key.id_or_name))):
removed_blobs = set(old_blob_lock_obj.get("active_blob_references", [])) - blob_list
old_blob_lock_obj["active_blob_references"] = list(blob_list)
if old_blob_lock_obj["old_blob_references"] is None:
old_blob_lock_obj["old_blob_references"] = list(removed_blobs)
else:
old_blob_refs = set(old_blob_lock_obj["old_blob_references"])
old_blob_refs.update(removed_blobs) # Add removed blobs
old_blob_refs -= blob_list # Remove active blobs
old_blob_lock_obj["old_blob_references"] = list(old_blob_refs)
old_blob_lock_obj["has_old_blob_references"] = bool(old_blob_lock_obj["old_blob_references"])
old_blob_lock_obj["is_stale"] = False
db.Put(old_blob_lock_obj)
else: # We need to create a new blob-lock-object
blob_lock_obj = db.Entity(db.Key("viur-blob-locks", db_obj.key.id_or_name))
blob_lock_obj["active_blob_references"] = list(blob_list)
blob_lock_obj["old_blob_references"] = []
blob_lock_obj["has_old_blob_references"] = False
blob_lock_obj["is_stale"] = False
db.Put(blob_lock_obj)

Which means that if two skeletons in different kinds have the same id_or_name, they will always overwrite each other's viur-block-locks entity.

In ViUR2 the complete key (including kindname) was used, so there were no conflicts: https://github.com/viur-framework/server/blob/e53c744834103a467a5f51b6b29db7aa531a6096/skeleton.py#L708


I think the probability that the same id is used twice with the randomly used ids is low. Especially if projects only have a few entities (< 1000). But it is not impossible. We should therefore change this implementation asap and use ancestors, for example.


The problem is to some way related to #1211. However, since I assume that in this case it is really caused by the error that is fixed in #1213, I would look at this separately.

@sveneberth sveneberth added bug(fix) Something isn't working or address a specific issue or vulnerability Priority: Critical This should be dealt with ASAP. It's blocking someone. labels Jul 12, 2024
@sveneberth sveneberth self-assigned this Jul 12, 2024
@phorward phorward added the viur-meeting Issues to discuss in the next ViUR meeting label Oct 10, 2024
@phorward phorward added this to the ViUR-core v3.7 milestone Oct 10, 2024
@phorward phorward removed the viur-meeting Issues to discuss in the next ViUR meeting label Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug(fix) Something isn't working or address a specific issue or vulnerability Priority: Critical This should be dealt with ASAP. It's blocking someone.
Projects
None yet
Development

No branches or pull requests

2 participants