You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "17krishna8 (via GitHub)" <gi...@apache.org> on 2026/10/16 15:49:51 UTC
[PR] Fall back to the default port when Url.parse cannot read the port [wicket]
17krishna8 opened a new pull request, #1673:
URL: https://github.com/apache/wicket/pull/1673
`Url.parse()` handed the port of a full URL to `Integer.parseInt()` without a guard, so a value such as `http://localhost:80x/` made the method throw `NumberFormatException` although nothing else about the URL is invalid, and `Url.parse()` is not documented to throw on malformed input at all.
The value is not always application controlled: `ServletWebRequest` reads the base URL of an ajax request from the `Wicket-Ajax-BaseURL` header or the `wicket-ajax-baseurl` parameter and passes it through `Url.parse()`, so a client that sends a malformed value turns an ajax request into a failed one. The websocket handshake parses the same parameter.
A port that cannot be parsed now falls back to the default port of the protocol, the same value a URL without a port gets. Everything else is parsed as before, and a valid port is still used.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fall back to the default port when Url.parse cannot read the port [wicket]
Posted by "reiern70 (via GitHub)" <gi...@apache.org>.
reiern70 commented on code in PR #1673:
URL: https://github.com/apache/wicket/pull/1673#discussion_r4300825615
##########
wicket-request/src/main/java/org/apache/wicket/request/Url.java:
##########
@@ -434,6 +434,29 @@ else if ("ftp".equals(protocol))
}
}
+ /**
+ * Parses the port part of the authority. A value that is not a number falls back to the
+ * default port of the protocol, so that a malformed port does not make the whole URL
+ * unparseable.
+ *
+ * @param port
+ * the port part of the authority
+ * @param protocol
+ * name of protocol
+ * @return the port, or the default port for the protocol if the value cannot be parsed
+ */
+ private static Integer parsePort(String port, String protocol)
+ {
+ try
+ {
+ return Integer.parseInt(port);
+ }
+ catch (NumberFormatException e)
+ {
+ return getDefaultPortForProtocol(protocol);
Review Comment:
This only catches ports that aren't an `int` at all. `:-1` and `:70000` are still taken as they are, so whether a bad port is replaced depends on whether it happens to fit an `int`; `parse23` locks in `99999999999` becoming 443 while `70000` stays 70000.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fall back to the default port when Url.parse cannot read the port [wicket]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #1673:
URL: https://github.com/apache/wicket/pull/1673#issuecomment-6228585181
Why fallback to a default value vs error? I'm concerned about having a default fallback on bad input could mask a broader issue.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Answer a 400 when the ajax base url port is not a number [wicket]
Posted by "17krishna8 (via GitHub)" <gi...@apache.org>.
17krishna8 commented on PR #1673:
URL: https://github.com/apache/wicket/pull/1673#issuecomment-6229813026
@mattrpav agreed, the fallback was the wrong call - `Url.parse()` keeps failing fast, and the request now gets a 400 instead.
@reiern70 thanks for the detailed review and the sketch. I have reworked the PR: `Url.parse()` is unchanged and now documents that it throws `NumberFormatException` for a port that is not a number, and `ServletWebRequest.getClientUrl()` answers a base url that cannot be parsed with the same 400 a missing one already gets, with your `getClientUrlAjaxWithMalformedBaseUrl` test next to `getClientUrlAjaxWithoutBaseUrl`.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Answer a 400 when the ajax base url port is not a number [wicket]
Posted by "17krishna8 (via GitHub)" <gi...@apache.org>.
17krishna8 commented on PR #1673:
URL: https://github.com/apache/wicket/pull/1673#issuecomment-6322846609
Thanks for testing it end to end and for digging into where `getClientUrl()` is first called during mapping - I had not looked past the exception being thrown either.
The description no longer claims the 400; it now spells out that until #1696 is merged, this only changes which exception reaches the exception mapper, the `onException` listeners and the log.
Once #1696 is in, I'll rebase and add `ajaxRequestWithMalformedBaseUrlAnswersBadRequest` (the `WicketTester` end-to-end test from your comment) so the request goes through the cycle.
For the record, the same limitation applies to the pre-existing 400 for a missing base url from WICKET-4841 - glad to see #1696 covers that path too.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org