Skip to content

Commit

Permalink
Fix race condition in version PUT endpoint
Browse files Browse the repository at this point in the history
If multiple concurrent calls to this endpoint occur, it's possible that
some data will be clobbered. This is fixed by using select_for_update to
lock the version before doing the diff and potential update of metadata.
  • Loading branch information
mvandenburgh committed Nov 20, 2023
1 parent 9dd155b commit cd6d26c
Showing 1 changed file with 14 additions and 8 deletions.
22 changes: 14 additions & 8 deletions dandiapi/api/views/version.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.db import transaction
from django.utils.decorators import method_decorator
from django_filters import rest_framework as filters
from drf_yasg.utils import no_body, swagger_auto_schema
Expand Down Expand Up @@ -99,16 +100,21 @@ def update(self, request, **kwargs):

# Strip away any computed fields from current and new metadata
new_metadata = Version.strip_metadata(new_metadata)
old_metadata = Version.strip_metadata(version.metadata)

# Only save version if metadata has actually changed
if (name, new_metadata) != (version.name, old_metadata):
version.name = name
version.metadata = new_metadata
version.status = Version.Status.PENDING
version.save()
with transaction.atomic():
# Re-query for the version, this time using a SELECT FOR UPDATE to
# ensure the object doesn't change out from under us.
locked_version = Version.objects.select_for_update().get(id=version.id)
old_metadata = Version.strip_metadata(locked_version.metadata)

serializer = VersionDetailSerializer(instance=version)
# Only save version if metadata has actually changed
if (name, new_metadata) != (locked_version.name, old_metadata):
locked_version.name = name
locked_version.metadata = new_metadata
locked_version.status = Version.Status.PENDING
locked_version.save()

serializer = VersionDetailSerializer(instance=locked_version)
return Response(serializer.data, status=status.HTTP_200_OK)

@swagger_auto_schema(
Expand Down

0 comments on commit cd6d26c

Please sign in to comment.