From bea27204c6c002e6c2dbb414f5cee76e0a51a53d Mon Sep 17 00:00:00 2001 From: nik Date: Thu, 7 Aug 2025 17:32:22 +0300 Subject: [PATCH] fix: BROS-91: Annotation Submission/Validation bypass race condition --- .../src/stores/Annotation/Annotation.js | 33 ++++-- web/libs/editor/src/stores/AppStore.js | 108 ++++++++++++------ web/libs/editor/src/tags/control/Choices.jsx | 1 + 3 files changed, 100 insertions(+), 42 deletions(-) diff --git a/web/libs/editor/src/stores/Annotation/Annotation.js b/web/libs/editor/src/stores/Annotation/Annotation.js index 77b35129fe58..e6479df81820 100644 --- a/web/libs/editor/src/stores/Annotation/Annotation.js +++ b/web/libs/editor/src/stores/Annotation/Annotation.js @@ -369,6 +369,8 @@ const _Annotation = types submissionStarted: 0, versions: {}, resultSnapshot: "", + // Flag to prevent concurrent validation calls + _isValidating: false, })) .volatile(() => isFF(FF_DEV_3391) @@ -586,17 +588,30 @@ const _Annotation = types }, validate() { - let ok = true; + // Prevent concurrent validation calls + if (self._isValidating) { + console.warn('Validation already in progress'); + return false; + } - self.traverseTree((node) => { - ok = node.validate?.(); - if (ok === false) { - return TRAVERSE_STOP; - } - }); + self._isValidating = true; + + try { + let ok = true; - // should be true or false - return ok ?? true; + self.traverseTree((node) => { + ok = node.validate?.(); + if (ok === false) { + return TRAVERSE_STOP; + } + }); + + // should be true or false + return ok ?? true; + } finally { + // Always reset the flag, even if an error occurs + self._isValidating = false; + } }, traverseTree(cb) { diff --git a/web/libs/editor/src/stores/AppStore.js b/web/libs/editor/src/stores/AppStore.js index 2c398149adb7..6348d5cfd794 100644 --- a/web/libs/editor/src/stores/AppStore.js +++ b/web/libs/editor/src/stores/AppStore.js @@ -120,6 +120,10 @@ export default types * Submitting task; used to prevent from duplicating requests */ isSubmitting: false, + /** + * Validating task; used to prevent concurrent validation calls during submission + */ + isValidating: false, /** * Flag for disable task in Label Studio */ @@ -592,63 +596,101 @@ export default types } function submitAnnotation() { - if (self.isSubmitting) return; + if (self.isSubmitting || self.isValidating) return; const entity = self.annotationStore.selected; const event = entity.exists ? "updateAnnotation" : "submitAnnotation"; - entity.beforeSend(); + // Set validation flag immediately to prevent concurrent validation calls + self.setFlags({ isValidating: true }); - if (!entity.validate()) return; + try { + entity.beforeSend(); - if (!isFF(FF_CUSTOM_SCRIPT)) { - entity.sendUserGenerate(); - } - handleSubmittingFlag(async () => { - if (isFF(FF_CUSTOM_SCRIPT)) { - await self.waitForDraftSubmission(); - const allowedToSave = await getEnv(self).events.invoke("beforeSaveAnnotation", self, entity, { event }); - if (allowedToSave && allowedToSave.some((x) => x === false)) return; + const isValid = entity.validate(); + + if (!isValid) { + // Reset validation flag on failure + self.setFlags({ isValidating: false }); + return; + } + + // Transition from validation to submission atomically + self.setFlags({ isValidating: false }); + if (!isFF(FF_CUSTOM_SCRIPT)) { entity.sendUserGenerate(); } - await getEnv(self).events.invoke(event, self, entity); - self.incrementQueuePosition(); - if (isFF(FF_CUSTOM_SCRIPT)) { + handleSubmittingFlag(async () => { + if (isFF(FF_CUSTOM_SCRIPT)) { + await self.waitForDraftSubmission(); + const allowedToSave = await getEnv(self).events.invoke("beforeSaveAnnotation", self, entity, { event }); + if (allowedToSave && allowedToSave.some((x) => x === false)) return; + + entity.sendUserGenerate(); + } + await getEnv(self).events.invoke(event, self, entity); + self.incrementQueuePosition(); + if (isFF(FF_CUSTOM_SCRIPT)) { + entity.dropDraft(); + } + }); + if (!isFF(FF_CUSTOM_SCRIPT)) { entity.dropDraft(); } - }); - if (!isFF(FF_CUSTOM_SCRIPT)) { - entity.dropDraft(); + } catch (error) { + // Reset validation flag on any error + self.setFlags({ isValidating: false }); + console.error('Submission error:', error); + throw error; } } function updateAnnotation(extraData) { - if (self.isSubmitting) return; + if (self.isSubmitting || self.isValidating) return; const entity = self.annotationStore.selected; - entity.beforeSend(); + // Set validation flag immediately to prevent concurrent validation calls + self.setFlags({ isValidating: true }); - if (!entity.validate()) return; + try { + entity.beforeSend(); - handleSubmittingFlag(async () => { - if (isFF(FF_CUSTOM_SCRIPT)) { - const allowedToSave = await getEnv(self).events.invoke("beforeSaveAnnotation", self, entity, { - event: "updateAnnotation", - }); - if (allowedToSave && allowedToSave.some((x) => x === false)) return; + const isValid = entity.validate(); + + if (!isValid) { + // Reset validation flag on failure + self.setFlags({ isValidating: false }); + return; } - await getEnv(self).events.invoke("updateAnnotation", self, entity, extraData); - self.incrementQueuePosition(); - if (isFF(FF_CUSTOM_SCRIPT)) { + + // Transition from validation to submission atomically + self.setFlags({ isValidating: false }); + + handleSubmittingFlag(async () => { + if (isFF(FF_CUSTOM_SCRIPT)) { + const allowedToSave = await getEnv(self).events.invoke("beforeSaveAnnotation", self, entity, { + event: "updateAnnotation", + }); + if (allowedToSave && allowedToSave.some((x) => x === false)) return; + } + await getEnv(self).events.invoke("updateAnnotation", self, entity, extraData); + self.incrementQueuePosition(); + if (isFF(FF_CUSTOM_SCRIPT)) { + entity.dropDraft(); + !entity.sentUserGenerate && entity.sendUserGenerate(); + } + }); + if (!isFF(FF_CUSTOM_SCRIPT)) { entity.dropDraft(); !entity.sentUserGenerate && entity.sendUserGenerate(); } - }); - if (!isFF(FF_CUSTOM_SCRIPT)) { - entity.dropDraft(); - !entity.sentUserGenerate && entity.sendUserGenerate(); + } catch (error) { + // Reset validation flag on any error + self.setFlags({ isValidating: false }); + console.error('Update annotation error:', error); + throw error; } } diff --git a/web/libs/editor/src/tags/control/Choices.jsx b/web/libs/editor/src/tags/control/Choices.jsx index 321ed80b4a4c..0a474992b1be 100644 --- a/web/libs/editor/src/tags/control/Choices.jsx +++ b/web/libs/editor/src/tags/control/Choices.jsx @@ -215,6 +215,7 @@ const Model = types return { validate() { if (!Super.validate() || (self.choice !== "multiple" && self.checkResultLength() > 1)) return false; + return true; }, checkResultLength() {