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.
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
