Andrea Cosentino created CAMEL-24652:
----------------------------------------
Summary: camel-servlet, camel-jetty, camel-platform-http-vertx -
consolidate the fileNameExtWhitelist check and tighten the extension match
Key: CAMEL-24652
URL: https://issues.apache.org/jira/browse/CAMEL-24652
Project: Camel
Issue Type: Improvement
Components: camel-http-common, camel-servlet, camel-jetty
Reporter: Andrea Cosentino
Assignee: Andrea Cosentino
Follow-up from the review of CAMEL-24427
(https://github.com/apache/camel/pull/26189), raised by davsclaus. Two points,
both deliberately left out of that PR to keep it focused.
*1. The whitelist match is loose, and CAMEL-24427 made it reachable.*
{{fileNameExtWhitelist}} was compared with {{whiteList.contains(ext)}}, a
substring test: with {{fileNameExtWhitelist=txt}}, an upload named {{evil.x}},
{{evil.t}} or {{evil.tx}} was accepted. CAMEL-24427 fixed the comparison in
camel-servlet and camel-jetty by splitting on {{,}} and comparing tokens
exactly, but {{VertxPlatformHttpConsumer}} still uses the substring form:
{code:java}
if (!fileNameExtWhitelist.equals("*") && !fileNameExtWhitelist.contains(ext)) {
accepted = false;
}
{code}
Separately, {{FileUtil.onlyExt}} runs in non-single mode, so it returns
everything after the *first* dot: {{archive.tar.gz}} yields {{tar.gz}}, and a
whitelist of {{gz}} rejects it. That affects all three components and is
arguably the more surprising of the two.
*2. There are three copies of the check.*
The premise of CAMEL-24427 was that three implementations disagreed about this
option. After it, camel-servlet and camel-jetty agree, but the logic exists
three times: inline in {{servlet/AttachmentHttpBinding}}, in
{{jetty12/AttachmentHttpBinding.isFileNameAccepted}}, and inline in
{{VertxPlatformHttpConsumer}}. Both bindings extend {{DefaultHttpBinding}} in
camel-http-common, which already owns {{fileNameExtWhitelist}} and has its own
copy of the check in {{populateAttachments}} - that is the natural home.
Lifting it there and having the subclasses and the vertx consumer call it would
leave one implementation and prevent the next drift.
Proposal: add a shared helper on {{DefaultHttpBinding}} (or a small util in
camel-http-base), have all call sites use it, decide whether {{onlyExt}} should
be single-mode for this check, and align {{VertxPlatformHttpConsumer}}.
Changing the extension parsing is a behaviour change and would need an
upgrade-guide entry.
_Claude Code on behalf of oscerd_
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
