gnodet-bot commented on code in PR #26268:
URL: https://github.com/apache/camel/pull/26268#discussion_r3991736968
##########
components/camel-sql/src/main/java/org/apache/camel/component/sql/stored/SqlStoredProducer.java:
##########
@@ -106,15 +106,29 @@ public void execute(StatementWrapper ps) throws
SQLException, DataAccessExceptio
private StatementWrapper createStatement(Exchange exchange) throws
SQLException {
String sql;
+ boolean runtime = false;
if (getEndpoint().isUseMessageBodyForTemplate()) {
sql = exchange.getIn().getBody(String.class);
+ runtime = true;
} else {
- String templateHeader =
exchange.getIn().getHeader(SqlStoredConstants.SQL_STORED_TEMPLATE,
String.class);
- sql = templateHeader != null ? templateHeader : resolvedTemplate;
+ String templateHeader = getEndpoint().isAllowTemplateFromHeader()
+ ?
exchange.getIn().getHeader(SqlStoredConstants.SQL_STORED_TEMPLATE,
String.class) : null;
+ if (templateHeader != null) {
+ sql = templateHeader;
+ runtime = true;
+ } else {
+ sql = resolvedTemplate;
+ }
}
try {
- sql = SqlHelper.resolveQuery(getEndpoint().getCamelContext(), sql,
null);
+ // A template taken at runtime from the message body
(useMessageBodyForTemplate) or from a header
+ // (CamelSqlStoredTemplate) is untrusted input, so it must not be
resolved as a file:/http:/classpath:
+ // resource (SqlHelper.resolveQuery -> ResourceHelper does that) -
resolve placeholders only. Only the
+ // endpoint-configured template is resolved as a resource, and
that already happens in doInit/doStart.
+ sql = runtime
+ ? SqlHelper.resolvePlaceholders(sql, null)
+ : SqlHelper.resolveQuery(getEndpoint().getCamelContext(),
sql, null);
Review Comment:
✅ Good fix. The `runtime` flag now correctly covers both the body path and
the header path. The comment is accurate — runtime templates (body or header)
are untrusted input and must not be resolved as resources.
One minor observation (pre-existing, not introduced by this commit): when
`runtime == false`, `sql` is `resolvedTemplate` — which was already resolved
through `SqlHelper.resolveQuery` in `doInit`/`doStart`. Calling `resolveQuery`
again is harmless but wasteful on every exchange. Could short-circuit to just
use `resolvedTemplate` directly. Not blocking.
##########
components/camel-sql/src/test/java/org/apache/camel/component/sql/stored/SqlStoredUseMessageBodyForTemplateResourceTest.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.camel.component.sql.stored;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.camel.CamelExecutionException;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.junit.jupiter.api.Test;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * When {@code useMessageBodyForTemplate=true} the message body is the
stored-procedure template text. Because the body
+ * is untrusted per-exchange input, it must be used verbatim and never
dereferenced as a {@code file:} / {@code http:} /
+ * {@code classpath:} resource - only the endpoint-configured template is
resolved as a resource (at route start).
+ */
+public class SqlStoredUseMessageBodyForTemplateResourceTest extends
CamelTestSupport {
+
+ private static final String INLINE_TEMPLATE = "SUBNUMBERS(INTEGER
:#num1,INTEGER :#num2,OUT INTEGER resultofsum)";
+
+ // Points at a real classpath resource holding a valid template. Before
the fix this body would have been loaded
+ // and executed; after the fix it is treated as literal (invalid) template
text.
+ private static final String RESOURCE_BODY =
"classpath:sql/bodyTemplateResource.sql";
+
+ private EmbeddedDatabase db;
+
+ @Override
+ public void doPreSetup() throws Exception {
+ db = new EmbeddedDatabaseBuilder()
+ .setName(getClass().getSimpleName())
+ .setType(EmbeddedDatabaseType.HSQL)
+ .addScript("sql/storedProcedureTest.sql").build();
+ }
+
+ @Override
+ public void doPostTearDown() throws Exception {
+ if (db != null) {
+ db.shutdown();
+ }
+ }
+
+ @Test
+ public void inlineBodyTemplateStillWorks() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:query");
+ mock.expectedMessageCount(1);
+
+ Map<String, Object> params = new HashMap<>();
+ params.put("num1", 3);
+ params.put("num2", 1);
+
+ template.requestBodyAndHeader("direct:query", INLINE_TEMPLATE,
SqlStoredConstants.SQL_STORED_PARAMETERS, params);
+
+ MockEndpoint.assertIsSatisfied(context);
+ assertEquals(Integer.valueOf(2),
mock.getExchanges().get(0).getIn().getBody(Map.class).get("resultofsum"));
+ }
+
+ @Test
+ public void schemePrefixedBodyIsNotResolvedAsResource() {
+ Map<String, Object> params = new HashMap<>();
+ params.put("num1", 3);
+ params.put("num2", 1);
+
+ // The body is a classpath: URI pointing at a valid template. It must
NOT be fetched and executed; instead the
+ // literal string is used as the template and fails to parse - which
is what proves the resource was not loaded.
+ assertThrows(CamelExecutionException.class,
Review Comment:
✅ Well-designed test. The `RESOURCE_BODY`
(`classpath:sql/bodyTemplateResource.sql`) points at a *valid* template on the
classpath. Before the fix, `resolveQuery` would load and execute it
successfully — so `assertThrows` would *fail*, proving the test is
mutation-resistant. After the fix, the literal string is used as template text
and fails to parse.
This is the right pattern — asserting on a parse failure of a literal
scheme-prefixed string, not on a generic execution error that fires regardless
of the code path (which is the problem with
`SqlStoredAllowTemplateFromHeaderTest`).
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -1864,3 +1864,28 @@ Documents carrying an internal DTD subset still parse.
To restore the previous behaviour and allow external entity resolution, set
the new
`allowExternalEntities` option to `true` on the data format or on the endpoint
(`smooks:config.xml?allowExternalEntities=true`).
+
+=== camel-sql, camel-sql-stored - the query/template override headers are gated
+
+A message header could override the endpoint-configured SQL by default,
letting an incoming message choose the
+executed statement:
+
+* `camel-sql`: the `CamelSqlQuery` header replaced the endpoint query.
+* `camel-sql-stored`: the `CamelSqlStoredTemplate` header replaced the
endpoint template, and its value was
+ resolved through `SqlHelper.resolveQuery`, which resolves `file:` / `http:`
/ `classpath:` resources.
+
+Both overrides are now disabled by default and must be opted into, matching
the `allowTemplateFromHeader`
+convention already used by the template components (camel-freemarker,
camel-velocity, camel-xslt, ...):
+
+* `camel-sql`: set `allowQueryFromHeader=true` to honour the `CamelSqlQuery`
header again.
+* `camel-sql-stored`: set `allowTemplateFromHeader=true` to honour the
`CamelSqlStoredTemplate` header again; a
+ header-supplied template is now resolved with property placeholders only,
never as a `file:` / `http:` resource.
+
+A route that relied on either header must set the corresponding option on the
endpoint.
+
+Additionally, `camel-sql-stored` with `useMessageBodyForTemplate=true` now
uses the message body verbatim as the
+stored-procedure template text and no longer resolves it through
`SqlHelper.resolveQuery`. A body beginning with
+`file:`, `http:`, `https:` or `classpath:` is therefore treated as literal
template text instead of being fetched
+as a resource, consistent with how `camel-sql` already treats the body under
`useMessageBodyForSql=true`. A route
+that relied on the body being a resource location must resolve it to the
template text before the `sql-stored`
+endpoint.
Review Comment:
💡 Minor wording alignment: this paragraph says `file:`, `http:`, `https:` or
`classpath:` — which is correct and complete. The earlier paragraph (line 1882)
says only `file:` / `http:`. Worth aligning both to the full list for
consistency (this was also flagged by @davsclaus on the first round).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
