From 9baa7c32bfe3cc6271e437a0e807449bebc1899e Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Fri, 2 Aug 2024 12:32:36 -0500 Subject: [PATCH] Add safety around most calls to formParam and/or queryParam, as they can throw if the request isn't formatted as expected, in ways that we may not want it to. --- .../qqq/api/javalin/QJavalinApiHandler.java | 15 +- .../javalin/QJavalinImplementation.java | 41 +-- .../qqq/backend/javalin/QJavalinUtils.java | 76 +++++ .../backend/javalin/QJavalinUtilsTest.java | 287 ++++++++++++++++++ 4 files changed, 387 insertions(+), 32 deletions(-) create mode 100644 qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinUtilsTest.java diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/javalin/QJavalinApiHandler.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/javalin/QJavalinApiHandler.java index 96625d79..8c541218 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/javalin/QJavalinApiHandler.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/javalin/QJavalinApiHandler.java @@ -458,11 +458,18 @@ public class QJavalinApiHandler ObjectUtils.ifNotNull(fieldsContainer.getRecordIdsField(), fields::add); for(QFieldMetaData field : fields) { - String queryParamValue = paramAccessor.apply(context, field.getName()); - if(queryParamValue != null) + try { - String backendName = ObjectUtils.requireConditionElse(field.getBackendName(), StringUtils::hasContent, field.getName()); - parameters.put(backendName, queryParamValue); + String value = paramAccessor.apply(context, field.getName()); + if(value != null) + { + String backendName = ObjectUtils.requireConditionElse(field.getBackendName(), StringUtils::hasContent, field.getName()); + parameters.put(backendName, value); + } + } + catch(Exception e) + { + LOG.info("Exception trying to process process input field", e); } } } diff --git a/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java b/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java index 61fdc389..3f4386ed 100644 --- a/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java +++ b/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java @@ -547,14 +547,17 @@ public class QJavalinImplementation } else { - String authorizationFormValue = context.formParam("Authorization"); - if(StringUtils.hasContent(authorizationFormValue)) + try { - processAuthorizationValue(authenticationContext, authorizationFormValue); + String authorizationFormValue = context.formParam("Authorization"); + if(StringUtils.hasContent(authorizationFormValue)) + { + processAuthorizationValue(authenticationContext, authorizationFormValue); + } } - else + catch(Exception e) { - LOG.debug("Neither [" + SESSION_ID_COOKIE_NAME + "] cookie nor [Authorization] header was present in request."); + LOG.info("Exception looking for Authorization formParam", e); } } @@ -565,7 +568,7 @@ public class QJavalinImplementation QSession session = authenticationModule.createSession(qInstance, authenticationContext); QContext.init(qInstance, session, null, input); - String tableVariant = StringUtils.hasContent(context.formParam("tableVariant")) ? context.formParam("tableVariant") : context.queryParam("tableVariant"); + String tableVariant = QJavalinUtils.getFormParamOrQueryParam(context, "tableVariant"); if(StringUtils.hasContent(tableVariant)) { JSONObject variant = new JSONObject(tableVariant); @@ -1183,11 +1186,7 @@ public class QJavalinImplementation PermissionsHelper.checkTablePermissionThrowing(countInput, TablePermissionSubType.READ); - filter = QJavalinUtils.stringQueryParam(context, "filter"); - if(!StringUtils.hasContent(filter)) - { - filter = context.formParam("filter"); - } + filter = QJavalinUtils.getQueryParamOrFormParam(context, "filter"); if(filter != null) { countInput.setFilter(JsonUtils.toObject(filter, QQueryFilter.class)); @@ -1256,11 +1255,7 @@ public class QJavalinImplementation PermissionsHelper.checkTablePermissionThrowing(queryInput, TablePermissionSubType.READ); - filter = QJavalinUtils.stringQueryParam(context, "filter"); - if(!StringUtils.hasContent(filter)) - { - filter = context.formParam("filter"); - } + filter = QJavalinUtils.getQueryParamOrFormParam(context, "filter"); if(filter != null) { QQueryFilter qQueryFilter = JsonUtils.toObject(filter, QQueryFilter.class); @@ -1540,23 +1535,13 @@ public class QJavalinImplementation PermissionsHelper.checkTablePermissionThrowing(exportInput, TablePermissionSubType.READ); - String fields = QJavalinUtils.stringQueryParam(context, "fields"); - if(!StringUtils.hasContent(fields)) - { - fields = context.formParam("fields"); - } - + String fields = QJavalinUtils.getQueryParamOrFormParam(context, "fields"); if(StringUtils.hasContent(fields)) { exportInput.setFieldNames(List.of(fields.split(","))); } - String filter = context.queryParam("filter"); - if(!StringUtils.hasContent(filter)) - { - filter = context.formParam("filter"); - } - + String filter = QJavalinUtils.getQueryParamOrFormParam(context, "filter"); if(StringUtils.hasContent(filter)) { exportInput.setQueryFilter(JsonUtils.toObject(filter, QQueryFilter.class)); diff --git a/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinUtils.java b/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinUtils.java index fb1a4681..daa3fe8b 100644 --- a/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinUtils.java +++ b/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinUtils.java @@ -102,4 +102,80 @@ public class QJavalinUtils return (null); } + + + + /*************************************************************************** + ** get a param value from either the form-body, or query string returning + ** the first one found, looking in that order, null if neither is found. + ** uses try-catch on reading each of those, as they apparently can throw! + ***************************************************************************/ + static String getFormParamOrQueryParam(Context context, String parameterName) + { + String value = null; + try + { + value = context.formParam(parameterName); + } + catch(Exception e) + { + //////////////// + // leave null // + //////////////// + } + + if(!StringUtils.hasContent(value)) + { + try + { + value = context.queryParam(parameterName); + } + catch(Exception e) + { + //////////////// + // leave null // + //////////////// + } + } + + return value; + } + + + + /*************************************************************************** + ** get a param value from either the query string, or form-body, returning + ** the first one found, looking in that order, null if neither is found. + ** uses try-catch on reading each of those, as they apparently can throw! + ***************************************************************************/ + static String getQueryParamOrFormParam(Context context, String parameterName) + { + String value = null; + try + { + value = context.queryParam(parameterName); + } + catch(Exception e) + { + //////////////// + // leave null // + //////////////// + } + + if(!StringUtils.hasContent(value)) + { + try + { + value = context.formParam(parameterName); + } + catch(Exception e) + { + //////////////// + // leave null // + //////////////// + } + } + + return value; + } } diff --git a/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinUtilsTest.java b/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinUtilsTest.java new file mode 100644 index 00000000..c8ffe0dd --- /dev/null +++ b/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinUtilsTest.java @@ -0,0 +1,287 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2024. Kingsrook, LLC + * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States + * contact@kingsrook.com + * https://github.com/Kingsrook/ + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package com.kingsrook.qqq.backend.javalin; + + +import java.io.InputStream; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.function.Supplier; +import io.javalin.http.Context; +import io.javalin.http.HandlerType; +import io.javalin.http.HttpStatus; +import jakarta.servlet.ServletOutputStream; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + + +/******************************************************************************* + ** Unit test for QJavalinUtils + *******************************************************************************/ +class QJavalinUtilsTest +{ + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void test() + { + //////////////////////////////////////////////////////////////////////////////////////////////// + // demonstrate that calling formParam or queryParam can throw (e.g., on our lame MockContext) // + //////////////////////////////////////////////////////////////////////////////////////////////// + assertThatThrownBy(() -> new MockContext(false, false).queryParam("foo")); + assertThatThrownBy(() -> new MockContext(false, false).formParam("foo")); + assertEquals("query:foo", new MockContext(true, false).queryParam("foo")); + assertEquals("form:foo", new MockContext(false, true).formParam("foo")); + + ////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // now demonstrate that calling these wrapping util methods avoid such exceptions (which was their intent.) // + // and, that when the context can return values, that the right ones are used // + ////////////////////////////////////////////////////////////////////////////////////////////////////////////// + assertNull(QJavalinUtils.getQueryParamOrFormParam(new MockContext(false, false), "foo")); + assertEquals("query:foo", QJavalinUtils.getQueryParamOrFormParam(new MockContext(true, false), "foo")); + assertEquals("form:foo", QJavalinUtils.getQueryParamOrFormParam(new MockContext(false, true), "foo")); + assertEquals("query:foo", QJavalinUtils.getQueryParamOrFormParam(new MockContext(true, true), "foo")); + + assertNull(QJavalinUtils.getFormParamOrQueryParam(new MockContext(false, false), "foo")); + assertEquals("form:foo", QJavalinUtils.getFormParamOrQueryParam(new MockContext(false, true), "foo")); + assertEquals("query:foo", QJavalinUtils.getFormParamOrQueryParam(new MockContext(true, false), "foo")); + assertEquals("form:foo", QJavalinUtils.getFormParamOrQueryParam(new MockContext(true, true), "foo")); + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + private static class MockContext implements Context + { + boolean returnsQueryParams; + boolean returnsFormParams; + + + + /******************************************************************************* + ** Constructor + ** + *******************************************************************************/ + public MockContext(boolean returnsQueryParams, boolean returnsFormParams) + { + this.returnsQueryParams = returnsQueryParams; + this.returnsFormParams = returnsFormParams; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @Nullable + @Override + public String queryParam(@NotNull String key) + { + if(this.returnsQueryParams) + { + return ("query:" + key); + } + + return Context.super.queryParam(key); + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @Nullable + @Override + public String formParam(@NotNull String key) + { + if(this.returnsFormParams) + { + return ("form:" + key); + } + + return Context.super.formParam(key); + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public HttpServletRequest req() + { + return null; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public HttpServletResponse res() + { + return null; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @Override + public T appAttribute(@NotNull String s) + { + return null; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public HandlerType handlerType() + { + return null; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public String matchedPath() + { + return ""; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public String endpointHandlerPath() + { + return ""; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public String pathParam(@NotNull String s) + { + return ""; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public Map pathParamMap() + { + return Map.of(); + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public ServletOutputStream outputStream() + { + return null; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @NotNull + @Override + public Context result(@NotNull InputStream inputStream) + { + return null; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @Nullable + @Override + public InputStream resultInputStream() + { + return null; + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @Override + public void future(@NotNull Supplier> supplier) + { + + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + @Override + public void redirect(@NotNull String s, @NotNull HttpStatus httpStatus) + { + + } + } +} \ No newline at end of file