From f96ce5fbafb5fbc5a2ebdf93ddce6f0c17faafb8 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 1 Dec 2025 13:54:05 -0800 Subject: [PATCH] Avoid error logging on bad SQL parameter values --- .../api/data/SQLParameterException.java | 29 +++++++++++++++++++ .../labkey/api/data/SqlExecutingSelector.java | 4 ++- api/src/org/labkey/api/data/Table.java | 10 ++++--- .../org/labkey/api/data/TableSelector.java | 2 +- .../org/labkey/api/util/ExceptionUtil.java | 8 +++++ .../api/util/SkipMothershipLogging.java | 10 ++----- .../org/labkey/query/QueryServiceImpl.java | 10 +++++-- 7 files changed, 57 insertions(+), 16 deletions(-) create mode 100644 api/src/org/labkey/api/data/SQLParameterException.java diff --git a/api/src/org/labkey/api/data/SQLParameterException.java b/api/src/org/labkey/api/data/SQLParameterException.java new file mode 100644 index 00000000000..0d13252128a --- /dev/null +++ b/api/src/org/labkey/api/data/SQLParameterException.java @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2011 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.data; + +import org.labkey.api.util.SkipMothershipLogging; + +/** + * Signals there was a problem with a SQL parameter, such as a conversion problem. + */ +public class SQLParameterException extends SQLGenerationException implements SkipMothershipLogging +{ + public SQLParameterException(String message) + { + super(message); + } +} diff --git a/api/src/org/labkey/api/data/SqlExecutingSelector.java b/api/src/org/labkey/api/data/SqlExecutingSelector.java index aeadad8bf99..71245935a1f 100644 --- a/api/src/org/labkey/api/data/SqlExecutingSelector.java +++ b/api/src/org/labkey/api/data/SqlExecutingSelector.java @@ -567,7 +567,9 @@ public void handleSqlException(SQLException e, @Nullable Connection conn) if (null != _sql) ExceptionUtil.decorateException(e, ExceptionUtil.ExceptionInfo.DialectSQL, _sql.toDebugString(), false); - throw getExceptionFramework().translate(getScope(), "ExecutingSelector", e); + RuntimeException translated = getExceptionFramework().translate(getScope(), "ExecutingSelector", e); + ExceptionUtil.copyDecorations(e, translated); + throw translated; } } } diff --git a/api/src/org/labkey/api/data/Table.java b/api/src/org/labkey/api/data/Table.java index e9bf16703fc..86962b4d295 100644 --- a/api/src/org/labkey/api/data/Table.java +++ b/api/src/org/labkey/api/data/Table.java @@ -19,6 +19,7 @@ import org.apache.commons.beanutils.BeanUtils; import org.apache.commons.beanutils.PropertyUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -50,6 +51,7 @@ import org.labkey.api.query.RuntimeValidationException; import org.labkey.api.security.User; import org.labkey.api.util.BaseScanner; +import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.MemTracker; @@ -571,7 +573,7 @@ Object getObject(ResultSet rs) throws SQLException // Standard SQLException catch block: log exception, query SQL, and params static void logException(@Nullable SQLFragment sql, @Nullable Connection conn, SQLException e, Level logLevel) { - if (SqlDialect.isCancelException(e)) + if (SqlDialect.isCancelException(e) || ExceptionUtil.isIgnorable(e)) { return; } @@ -585,7 +587,7 @@ static void logException(@Nullable SQLFragment sql, @Nullable Connection conn, S String trim = sql.getSQL().trim(); // Treat a ConstraintException during INSERT/UPDATE as a WARNING. Treat all other SQLExceptions as an ERROR. - if (RuntimeSQLException.isConstraintException(e) && (StringUtils.startsWithIgnoreCase(trim, "INSERT") || StringUtils.startsWithIgnoreCase(trim, "UPDATE"))) + if (RuntimeSQLException.isConstraintException(e) && (Strings.CI.startsWith(trim, "INSERT") || Strings.CI.startsWith(trim, "UPDATE"))) { // Log this ConstraintException if log Level is WARN (the default) or lower. Skip logging for callers that request just ERRORs. if (Level.WARN.isMoreSpecificThan(logLevel)) @@ -1409,7 +1411,7 @@ public void testSelect() throws SQLException //noinspection EmptyTryBlock,UnusedDeclaration try (ResultSet rs = new TableSelector(tinfo).getResultSet()){} - Map[] maps = new TableSelector(tinfo).getMapArray(); + Map[] maps = new TableSelector(tinfo).getMapArray(); assertNotNull(maps); Principal[] principals = new TableSelector(tinfo).getArray(Principal.class); @@ -1739,7 +1741,7 @@ public static ParameterMapStatement deleteStatement(Connection conn, TableInfo t // Domain domain = tableDelete.getDomain(); - DomainKind domainKind = tableDelete.getDomainKind(); + DomainKind domainKind = tableDelete.getDomainKind(); if (null != domain && null != domainKind && StringUtils.isEmpty(domainKind.getStorageSchemaName())) { if (!d.isPostgreSQL() && !d.isSqlServer()) diff --git a/api/src/org/labkey/api/data/TableSelector.java b/api/src/org/labkey/api/data/TableSelector.java index f64a43fb55e..dc2bde07a69 100644 --- a/api/src/org/labkey/api/data/TableSelector.java +++ b/api/src/org/labkey/api/data/TableSelector.java @@ -396,7 +396,7 @@ public Results getResultsAsync(final boolean cache, final boolean scrollable, Ht } /** - * Setting this options asks the TableSelector to add additional display columns to the generated SQL, as well + * Setting this option asks the TableSelector to add additional display columns to the generated SQL, as well * as forcing the results to be sorted. * @return this diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index 3649564a0ba..4e2555cf644 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -106,6 +106,14 @@ public class ExceptionUtil */ private static final Map EXCEPTION_TALLIES = Collections.synchronizedMap(new HashMap<>()); + public static void copyDecorations(Throwable source, Throwable target) + { + for (Map.Entry, String> entry : getExceptionDecorations(source).entrySet()) + { + decorateException(target, entry.getKey(), entry.getValue(), false); + } + } + private static class ExceptionTally { /** Total number of times the exception has happened */ diff --git a/api/src/org/labkey/api/util/SkipMothershipLogging.java b/api/src/org/labkey/api/util/SkipMothershipLogging.java index 674e41cb756..414b06d3a5e 100644 --- a/api/src/org/labkey/api/util/SkipMothershipLogging.java +++ b/api/src/org/labkey/api/util/SkipMothershipLogging.java @@ -16,13 +16,9 @@ package org.labkey.api.util; -/* -* User: adam -* Date: Aug 10, 2009 -* Time: 2:03:50 PM -*/ - -// Exceptions implement this interface to tell mothership not to log them +/** + * Exceptions implement this interface as a tag to tell mothership not to log them + */ public interface SkipMothershipLogging { } diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 009b239df58..0883a1ec48d 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -23,6 +23,7 @@ import org.apache.commons.collections4.MultiValuedMap; import org.apache.commons.collections4.SetValuedMap; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.xmlbeans.XmlError; @@ -65,7 +66,7 @@ import org.labkey.api.data.ResultsImpl; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; -import org.labkey.api.data.SQLGenerationException; +import org.labkey.api.data.SQLParameterException; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; import org.labkey.api.data.SqlSelector; @@ -120,6 +121,7 @@ import org.labkey.api.util.CSRFUtil; import org.labkey.api.util.ConfigurationException; import org.labkey.api.util.ContainerContext; +import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.Pair; @@ -1360,7 +1362,7 @@ public MultiValuedMap load(Stream { // Note: Can't use standard filter (getFilter(suffix)) below since we must allow ".qview.xml" return unmodifiable(resources - .filter(resource -> StringUtils.endsWithIgnoreCase(resource.getName(), CustomViewXmlReader.XML_FILE_EXTENSION)) + .filter(resource -> Strings.CI.endsWith(resource.getName(), CustomViewXmlReader.XML_FILE_EXTENSION)) .map(ModuleCustomViewDef::new) .collect(LabKeyCollectors.toMultiValuedMap(def -> def.getPath().getParent(), def -> def))); } @@ -2683,7 +2685,9 @@ public void bindNamedParameters(SQLFragment frag, @Nullable Map } catch (ConversionException e) { - throw new RuntimeSQLException(new SQLGenerationException(ConvertHelper.getStandardConversionErrorMessage(value, p.getName(), p.getJdbcType().getJavaClass()))); + SQLParameterException paramException = new SQLParameterException(ConvertHelper.getStandardConversionErrorMessage(value, p.getName(), p.getJdbcType().getJavaClass())); + ExceptionUtil.decorateException(paramException, ExceptionUtil.ExceptionInfo.SkipMothershipLogging, "true", true); + throw new RuntimeSQLException(paramException); } } }