[
https://issues.apache.org/jira/browse/ARTEMIS-4926?focusedWorklogId=953706&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-953706
]
ASF GitHub Bot logged work on ARTEMIS-4926:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 22/Jan/25 17:51
Start Date: 22/Jan/25 17:51
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #5452:
URL: https://github.com/apache/activemq-artemis/pull/5452#discussion_r1925735128
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/uri/URISupport.java:
##########
@@ -140,12 +141,24 @@ public static boolean containsQuery(SimpleString uri) {
return uri.contains('?');
}
- private static void parseParameters(Map<String, String> rc, String[]
parameters) {
+ protected static void parseParameters(Map<String, String> rc, String[]
parameters) {
for (String parameter : parameters) {
int p = parameter.indexOf("=");
if (p >= 0) {
- String name = URLDecoder.decode(parameter.substring(0, p),
StandardCharsets.UTF_8);
- String value = URLDecoder.decode(parameter.substring(p + 1),
StandardCharsets.UTF_8);
+ String name;
+ String value;
+ try {
+ name = URLDecoder.decode(parameter.substring(0, p),
StandardCharsets.UTF_8);
+ } catch (IllegalArgumentException e) {
+
ActiveMQUtilLogger.LOGGER.unableToParseURLParameterName(parameter.substring(0,
p), e);
+ continue;
Review Comment:
Catching and continuing does not actually seem like an improvement to me at
all. Personally, I would want it to throw rather than just continue and perhaps
do something quite different than I had perhaps intended.
The original complaint seems to be more that this sub-method can throw IAE
and the calling methods dont handle that explicitly, or declare so. Feels like
they could as easily just declare it throws IAE, or even perhaps wrap with
URISyntaxException.
If taking that approach, this could just call _parseParameters(URI uri)_ per
the original report and check behviour, rather than exposing this private
method.
Issue Time Tracking
-------------------
Worklog Id: (was: 953706)
Time Spent: 20m (was: 10m)
> IllegalArgumentException in UriSupport.parseParameters
> ------------------------------------------------------
>
> Key: ARTEMIS-4926
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4926
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Reporter: Ekaterina Zilotina
> Assignee: Justin Bertram
> Priority: Major
> Labels: pull-request-available
> Attachments: UriSupportFuzzer.java.txt,
> UriSupportcrash-00152a429040cf0bb95bdce6422303498a30631a,
> UriSupportcrash-084e9380bd54a4f1eba0131ca1d67f2720c76025,
> UriSupportcrash-90b1ee0ba36f0cae32ac20469ce0d3ddcfa8e5fa,
> UriSupportcrash-a520043b41390db8ef10a6675f43ecf6faa7e859,
> UriSupportcrash-b46a887ae8b7dea48921f85c09f35694d9f502b9, fuzz_state.txt
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Function *URLDecoder.decode()* uses in lines
> [147|https://github.com/apache/activemq-artemis/blob/b4d3a776499cb3ef9a350107faa998c81b20c3e6/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/uri/URISupport.java#L147]
> and
> [148|https://github.com/apache/activemq-artemis/blob/b4d3a776499cb3ef9a350107faa998c81b20c3e6/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/uri/URISupport.java#L148]
> (URISupport.java) and can produce {*}IllegalArgumentException{*}, which
> won't be catched when function *UriSupport.parseParameters()* works.
> This error was found with pure *UriSupport.parseParameters(URI uri)* fuzz
> testing and may be it does not pose a risk to artemis, but this is important
> to me, because in this code area there isn't any handling of it.
> crash samples, fuzz test and part of jazzer log are below
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
