[
https://issues.apache.org/jira/browse/ARTEMIS-4926?focusedWorklogId=953726&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-953726
]
ASF GitHub Bot logged work on ARTEMIS-4926:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 22/Jan/25 19:29
Start Date: 22/Jan/25 19:29
Worklog Time Spent: 10m
Work Description: tabish121 commented on code in PR #5452:
URL: https://github.com/apache/activemq-artemis/pull/5452#discussion_r1925857044
##########
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:
This looks good to me. I had similar thoughts on the original changes as
Robbie did, I would generally expect things to fail than to suppress errors and
lead people to think configuration was applied when it was not.
Issue Time Tracking
-------------------
Worklog Id: (was: 953726)
Time Spent: 40m (was: 0.5h)
> Throw checked URISyntaxException on bad URL
> -------------------------------------------
>
> 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: 40m
> 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
