forked from CGM_Public/pretix_original
Orders API: Fix race condition in voucher redemption (Z#23230391) (#6067)
The old code relied on the `Voucher.redeemed` value obtained *before* the lock was taken, not afterwards. The change in services/orders.py is functionally pointless, but it makes the pattern of "fill availability only after lock" clearer and might avoid introducing similar bugs in the future.
This commit is contained in:
@@ -1416,6 +1416,7 @@ class OrderCreateSerializer(I18nAwareModelSerializer):
|
|||||||
qa = QuotaAvailability()
|
qa = QuotaAvailability()
|
||||||
qa.queue(*[q for q, d in quota_diff_for_locking.items() if d > 0])
|
qa.queue(*[q for q, d in quota_diff_for_locking.items() if d > 0])
|
||||||
qa.compute()
|
qa.compute()
|
||||||
|
v_avail = {}
|
||||||
|
|
||||||
# These are not technically correct as diff use due to the time offset applied above, so let's prevent accidental
|
# These are not technically correct as diff use due to the time offset applied above, so let's prevent accidental
|
||||||
# use further down
|
# use further down
|
||||||
@@ -1445,11 +1446,13 @@ class OrderCreateSerializer(I18nAwareModelSerializer):
|
|||||||
|
|
||||||
voucher_usage[v] += 1
|
voucher_usage[v] += 1
|
||||||
if voucher_usage[v] > 0:
|
if voucher_usage[v] > 0:
|
||||||
redeemed_in_carts = CartPosition.objects.filter(
|
if v not in v_avail:
|
||||||
Q(voucher=pos_data['voucher']) & Q(event=self.context['event']) & Q(expires__gte=now_dt)
|
v.refresh_from_db(fields=['redeemed'])
|
||||||
).exclude(pk__in=[cp.pk for cp in delete_cps])
|
redeemed_in_carts = CartPosition.objects.filter(
|
||||||
v_avail = v.max_usages - v.redeemed - redeemed_in_carts.count()
|
Q(voucher=v) & Q(event=self.context['event']) & Q(expires__gte=now_dt)
|
||||||
if v_avail < voucher_usage[v]:
|
).exclude(pk__in=[cp.pk for cp in delete_cps])
|
||||||
|
v_avail[v] = v.max_usages - v.redeemed - redeemed_in_carts.count()
|
||||||
|
if v_avail[v] < voucher_usage[v]:
|
||||||
errs[i]['voucher'] = [
|
errs[i]['voucher'] = [
|
||||||
'The voucher has already been used the maximum number of times.'
|
'The voucher has already been used the maximum number of times.'
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -727,8 +727,6 @@ def _check_positions(event: Event, now_dt: datetime, time_machine_now_dt: dateti
|
|||||||
_check_date(event, time_machine_now_dt)
|
_check_date(event, time_machine_now_dt)
|
||||||
|
|
||||||
products_seen = Counter()
|
products_seen = Counter()
|
||||||
q_avail = Counter()
|
|
||||||
v_avail = Counter()
|
|
||||||
v_usages = Counter()
|
v_usages = Counter()
|
||||||
v_budget = {}
|
v_budget = {}
|
||||||
deleted_positions = set()
|
deleted_positions = set()
|
||||||
@@ -793,6 +791,9 @@ def _check_positions(event: Event, now_dt: datetime, time_machine_now_dt: dateti
|
|||||||
shared_lock_objects=[event]
|
shared_lock_objects=[event]
|
||||||
)
|
)
|
||||||
|
|
||||||
|
q_avail = Counter()
|
||||||
|
v_avail = Counter()
|
||||||
|
|
||||||
# Check maximum order size
|
# Check maximum order size
|
||||||
limit = min(int(event.settings.max_items_per_order), settings.PRETIX_MAX_ORDER_SIZE)
|
limit = min(int(event.settings.max_items_per_order), settings.PRETIX_MAX_ORDER_SIZE)
|
||||||
if sum(1 for cp in sorted_positions if not cp.addon_to) > limit:
|
if sum(1 for cp in sorted_positions if not cp.addon_to) > limit:
|
||||||
|
|||||||
Reference in New Issue
Block a user