Skip to content

fix: BROS-91: Annotation Submission/Validation bypass race condition #8111

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: BROS-91: Annotation Submission/Validation bypass race condition #8111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions web/libs/editor/src/stores/Annotation/Annotation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,8 @@ const _Annotation = types
submissionStarted: 0,
versions: {},
resultSnapshot: "",
// Flag to prevent concurrent validation calls
_isValidating: false,
}))
.volatile(() =>
isFF(FF_DEV_3391)
Expand Down Expand Up @@ -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) {
Expand Down
108 changes: 75 additions & 33 deletions web/libs/editor/src/stores/AppStore.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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;
}
}

Expand Down
1 change: 1 addition & 0 deletions web/libs/editor/src/tags/control/Choices.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ const Model = types
return {
validate() {
if (!Super.validate() || (self.choice !== "multiple" && self.checkResultLength() > 1)) return false;
return true;
},

checkResultLength() {
Expand Down
Loading