From cd6d26c9ed21dab1aee5c5e56071d97cb183b555 Mon Sep 17 00:00:00 2001 From: Mike VanDenburgh Date: Mon, 20 Nov 2023 17:22:50 -0500 Subject: [PATCH] Fix race condition in version PUT endpoint 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. --- dandiapi/api/views/version.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/dandiapi/api/views/version.py b/dandiapi/api/views/version.py index 62a36094d..beb091074 100644 --- a/dandiapi/api/views/version.py +++ b/dandiapi/api/views/version.py @@ -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 @@ -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(