From 6fb546733c3798be3d943893274768c4aec6bda6 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 11 Nov 2025 09:24:11 +0000 Subject: [PATCH] fix(forms): Resolve race condition in form handlers Refactored SubmitFormHandler and AjaxSubmitFormHandler to correctly handle asynchronous form processing. The response logic is now executed within the Promise callbacks, ensuring that the response is sent only after the form processing is complete. --- .../forms/handler/AjaxSubmitFormHandler.java | 38 ++++++++++++------- .../forms/handler/SubmitFormHandler.java | 20 ++++++---- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/condation/cms/modules/forms/handler/AjaxSubmitFormHandler.java b/src/main/java/com/condation/cms/modules/forms/handler/AjaxSubmitFormHandler.java index f0e271a..9009233 100644 --- a/src/main/java/com/condation/cms/modules/forms/handler/AjaxSubmitFormHandler.java +++ b/src/main/java/com/condation/cms/modules/forms/handler/AjaxSubmitFormHandler.java @@ -63,12 +63,12 @@ public boolean handle(Request request, Response response, Callback callback) thr FormsHandling formHandling = new FormsHandling(hookSystem); - final AtomicReference formResponse = new AtomicReference<>(); try { if (MimeTypes.Type.FORM_ENCODED.is(contentType)) { FormFields.onFields(request, StandardCharsets.UTF_8, new Promise.Invocable() { @Override public void succeeded(Fields fields) { + FormResponse formResponse; try { final String formName = fields.get("form").getValue(); var form = FormsLifecycleExtension.FORMSCONFIG.findForm(formName).get(); @@ -78,16 +78,21 @@ public void succeeded(Fields fields) { } return field; }); - formResponse.set(new FormResponse(false)); + formResponse = new FormResponse(false); + response.setStatus(HttpStatus.OK_200); } catch (FormHandlingException fhe) { log.error(null, fhe); - formResponse.set(new FormResponse(true)); + formResponse = new FormResponse(true); + response.setStatus(HttpStatus.BAD_REQUEST_400); } + Content.Sink.write(response, true, GSON.toJson(formResponse), callback); } @Override public void failed(Throwable x) { - formResponse.set(new FormResponse(true)); + var formResponse = new FormResponse(true); + response.setStatus(HttpStatus.BAD_REQUEST_400); + Content.Sink.write(response, true, GSON.toJson(formResponse), callback); } }); } else if (contentType.startsWith(MimeTypes.Type.MULTIPART_FORM_DATA.asString())) { @@ -98,11 +103,14 @@ public void failed(Throwable x) { parser.parse(request, new Promise.Invocable() { @Override public void failed(Throwable x) { - formResponse.set(new FormResponse(true)); + var formResponse = new FormResponse(true); + response.setStatus(HttpStatus.BAD_REQUEST_400); + Content.Sink.write(response, true, GSON.toJson(formResponse), callback); } @Override public void succeeded(MultiPartFormData.Parts parts) { + FormResponse formResponse; try { String formName = parts.getFirst("form").getContentAsString(StandardCharsets.UTF_8); @@ -114,26 +122,28 @@ public void succeeded(MultiPartFormData.Parts parts) { return field; }); - formResponse.set(new FormResponse(false)); - + formResponse = new FormResponse(false); + response.setStatus(HttpStatus.OK_200); } catch (FormHandlingException fhe) { log.error(null, fhe); - formResponse.set(new FormResponse(true)); + formResponse = new FormResponse(true); + response.setStatus(HttpStatus.BAD_REQUEST_400); } + Content.Sink.write(response, true, GSON.toJson(formResponse), callback); } }); + } else { + var formResponse = new FormResponse(true); + response.setStatus(HttpStatus.BAD_REQUEST_400); + Content.Sink.write(response, true, GSON.toJson(formResponse), callback); } } catch (Exception e) { log.error("error processing form", e); - } - - if (formResponse.get().error()) { + var formResponse = new FormResponse(true); response.setStatus(HttpStatus.BAD_REQUEST_400); - } else { - response.setStatus(HttpStatus.OK_200); + Content.Sink.write(response, true, GSON.toJson(formResponse), callback); } - Content.Sink.write(response, true, GSON.toJson(formResponse.get()), callback); return true; } diff --git a/src/main/java/com/condation/cms/modules/forms/handler/SubmitFormHandler.java b/src/main/java/com/condation/cms/modules/forms/handler/SubmitFormHandler.java index 9c25885..918cd99 100644 --- a/src/main/java/com/condation/cms/modules/forms/handler/SubmitFormHandler.java +++ b/src/main/java/com/condation/cms/modules/forms/handler/SubmitFormHandler.java @@ -75,6 +75,7 @@ public boolean handle(Request request, Response response, Callback callback) thr public void failed(Throwable x) { response.getHeaders().add("Location", FormsLifecycleExtension.FORMSCONFIG.getRedirects().getError()); response.setStatus(HttpStatus.MOVED_TEMPORARILY_302); + callback.succeeded(); } @Override @@ -101,10 +102,11 @@ public void succeeded(Fields fields) { response.getHeaders().add("Location", FormsLifecycleExtension.FORMSCONFIG.getRedirects().getError()); response.setStatus(HttpStatus.MOVED_TEMPORARILY_302); } + } finally { + callback.succeeded(); } } }); - return true; } else if (contentType.startsWith(MimeTypes.Type.MULTIPART_FORM_DATA.asString())) { String boundary = MultiPart.extractBoundary(contentType); MultiPartFormData.Parser parser = new MultiPartFormData.Parser(boundary); @@ -114,7 +116,7 @@ public void succeeded(Fields fields) { public void failed(Throwable x) { response.getHeaders().add("Location", FormsLifecycleExtension.FORMSCONFIG.getRedirects().getError()); response.setStatus(HttpStatus.MOVED_TEMPORARILY_302); - + callback.succeeded(); } @Override @@ -143,19 +145,23 @@ public void succeeded(MultiPartFormData.Parts parts) { response.getHeaders().add("Location", FormsLifecycleExtension.FORMSCONFIG.getRedirects().getError()); response.setStatus(HttpStatus.MOVED_TEMPORARILY_302); } + } finally { + callback.succeeded(); } } }); - return true; + } else { + response.getHeaders().add("Location", FormsLifecycleExtension.FORMSCONFIG.getRedirects().getError()); + response.setStatus(HttpStatus.MOVED_TEMPORARILY_302); + callback.succeeded(); } } catch (Exception e) { log.error("error processing form", e); + response.getHeaders().add("Location", FormsLifecycleExtension.FORMSCONFIG.getRedirects().getError()); + response.setStatus(HttpStatus.MOVED_TEMPORARILY_302); + callback.succeeded(); } - - response.getHeaders().add("Location", FormsLifecycleExtension.FORMSCONFIG.getRedirects().getError()); - response.setStatus(HttpStatus.MOVED_TEMPORARILY_302); - callback.succeeded(); return true; } }