[
https://issues.apache.org/jira/browse/CAMEL-24652?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18121893#comment-18121893
]
Andrea Cosentino commented on CAMEL-24652:
------------------------------------------
Backport PRs: https://github.com/apache/camel/pull/27267 (camel-4.22.x) and
https://github.com/apache/camel/pull/27268 (camel-4.18.x), together covering
CAMEL-24650 and CAMEL-24652; upgrade-guide entries for 4.22.2 and 4.18.5 in
https://github.com/apache/camel/pull/27269. fixVersions to be added once they
merge.
_Claude Code on behalf of oscerd_
> 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-jetty, camel-servlet
> Reporter: Andrea Cosentino
> Assignee: Andrea Cosentino
> Priority: Minor
> Fix For: 4.23.0
>
>
> 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)
