snoopdave commented on code in PR #236:
URL: https://github.com/apache/roller/pull/236#discussion_r4492927000
##########
app/src/main/java/org/apache/roller/weblogger/business/pings/WeblogUpdatePinger.java:
##########
@@ -114,42 +141,234 @@ public static PingResult sendPing(PingTarget pingTarget,
Weblog website) throws
LOGGER.debug("Executing ping to '" + pingTargetUrl + "' for
website '" + websiteUrl + "' (" + website.getName() + ")" +
(variantOptions.isEmpty() ? "" : " with variant options " + variantOptions));
}
- // Send the ping.
- XmlRpcClientConfigImpl config = new XmlRpcClientConfigImpl();
- config.setServerURL(new URL(pingTargetUrl));
- XmlRpcClient client = new XmlRpcClient();
- client.setConfig(config);
- PingResult pingResult =
parseResult(client.execute("weblogUpdates.ping", params.toArray()));
+ // Send the ping and read the response.
+ byte[] response = exchange(pingTargetUrl, buildRequest(params));
+ PingResult pingResult = parseResponse(response);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Ping result is: " + pingResult);
}
return pingResult;
}
- private static PingResult parseResult(Object obj) {
- // Deal with the fact that some buggy ping targets may not respond
with the proper struct type.
- if (obj == null) {
- return new PingResult(null,null);
+ /**
+ * Build the XML-RPC <code>weblogUpdates.ping</code> request body for the
+ * given string parameters.
+ */
+ static String buildRequest(List<String> params) {
+ StringBuilder sb = new StringBuilder(256);
+ sb.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
+
sb.append("<methodCall><methodName>").append(PING_METHOD_NAME).append("</methodName>");
+ sb.append("<params>");
+ for (String param : params) {
+ sb.append("<param><value><string>");
+ xmlEscape(sb, param);
+ sb.append("</string></value></param>");
+ }
+ sb.append("</params></methodCall>");
+ return sb.toString();
+ }
+
+ /**
+ * POST the request body to the ping target and return the response bytes,
+ * capped at {@link #MAX_RESPONSE_BYTES}.
+ */
+ private static byte[] exchange(String pingTargetUrl, String requestBody)
throws IOException {
+ URL url = new URL(pingTargetUrl);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+ try {
+ conn.setRequestMethod("POST");
+ conn.setConnectTimeout(CONNECT_TIMEOUT_MS);
+ conn.setReadTimeout(READ_TIMEOUT_MS);
+ conn.setDoOutput(true);
+ conn.setRequestProperty("Content-Type", "text/xml; charset=UTF-8");
+ conn.setRequestProperty("User-Agent", "Apache Roller");
+
+ byte[] body = requestBody.getBytes(StandardCharsets.UTF_8);
+ conn.setFixedLengthStreamingMode(body.length);
+ try (OutputStream out = conn.getOutputStream()) {
+ out.write(body);
+ }
+
+ int status = conn.getResponseCode();
+ InputStream in = (status >= 200 && status < 300) ?
conn.getInputStream() : conn.getErrorStream();
Review Comment:
🤖Claude: fixed by throwing an IOException on a non-2xx HTTP status (in
exchange()) instead of reading and parsing the error body, so a
failing/erroring target is now treated as a failed ping and retried by the
queue processor. Added nonSuccessHttpStatusThrows and
successHttpStatusReturnsBody tests using a loopback HTTP server. (fe9f9f2c9)
##########
app/src/main/java/org/apache/roller/weblogger/business/pings/WeblogUpdatePinger.java:
##########
@@ -114,42 +141,234 @@ public static PingResult sendPing(PingTarget pingTarget,
Weblog website) throws
LOGGER.debug("Executing ping to '" + pingTargetUrl + "' for
website '" + websiteUrl + "' (" + website.getName() + ")" +
(variantOptions.isEmpty() ? "" : " with variant options " + variantOptions));
}
- // Send the ping.
- XmlRpcClientConfigImpl config = new XmlRpcClientConfigImpl();
- config.setServerURL(new URL(pingTargetUrl));
- XmlRpcClient client = new XmlRpcClient();
- client.setConfig(config);
- PingResult pingResult =
parseResult(client.execute("weblogUpdates.ping", params.toArray()));
+ // Send the ping and read the response.
+ byte[] response = exchange(pingTargetUrl, buildRequest(params));
+ PingResult pingResult = parseResponse(response);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Ping result is: " + pingResult);
}
return pingResult;
}
- private static PingResult parseResult(Object obj) {
- // Deal with the fact that some buggy ping targets may not respond
with the proper struct type.
- if (obj == null) {
- return new PingResult(null,null);
+ /**
+ * Build the XML-RPC <code>weblogUpdates.ping</code> request body for the
+ * given string parameters.
+ */
+ static String buildRequest(List<String> params) {
+ StringBuilder sb = new StringBuilder(256);
+ sb.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
+
sb.append("<methodCall><methodName>").append(PING_METHOD_NAME).append("</methodName>");
+ sb.append("<params>");
+ for (String param : params) {
+ sb.append("<param><value><string>");
+ xmlEscape(sb, param);
+ sb.append("</string></value></param>");
+ }
+ sb.append("</params></methodCall>");
+ return sb.toString();
+ }
+
+ /**
+ * POST the request body to the ping target and return the response bytes,
+ * capped at {@link #MAX_RESPONSE_BYTES}.
+ */
+ private static byte[] exchange(String pingTargetUrl, String requestBody)
throws IOException {
+ URL url = new URL(pingTargetUrl);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
Review Comment:
🤖Claude: fixed by checking that url.openConnection() returns an
HttpURLConnection and throwing an IOException otherwise, so a non-HTTP target
(file:/ftp:/etc.) flows through the normal ping-error handling instead of
escaping as a ClassCastException. Added nonHttpTargetThrowsIoException.
(fe9f9f2c9)
##########
app/src/main/java/org/apache/roller/weblogger/business/pings/WeblogUpdatePinger.java:
##########
@@ -114,42 +141,234 @@
LOGGER.debug("Executing ping to '" + pingTargetUrl + "' for
website '" + websiteUrl + "' (" + website.getName() + ")" +
(variantOptions.isEmpty() ? "" : " with variant options " + variantOptions));
}
- // Send the ping.
- XmlRpcClientConfigImpl config = new XmlRpcClientConfigImpl();
- config.setServerURL(new URL(pingTargetUrl));
- XmlRpcClient client = new XmlRpcClient();
- client.setConfig(config);
- PingResult pingResult =
parseResult(client.execute("weblogUpdates.ping", params.toArray()));
+ // Send the ping and read the response.
+ byte[] response = exchange(pingTargetUrl, buildRequest(params));
+ PingResult pingResult = parseResponse(response);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Ping result is: " + pingResult);
}
return pingResult;
}
- private static PingResult parseResult(Object obj) {
- // Deal with the fact that some buggy ping targets may not respond
with the proper struct type.
- if (obj == null) {
- return new PingResult(null,null);
+ /**
+ * Build the XML-RPC <code>weblogUpdates.ping</code> request body for the
+ * given string parameters.
+ */
+ static String buildRequest(List<String> params) {
+ StringBuilder sb = new StringBuilder(256);
+ sb.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
+
sb.append("<methodCall><methodName>").append(PING_METHOD_NAME).append("</methodName>");
+ sb.append("<params>");
+ for (String param : params) {
+ sb.append("<param><value><string>");
+ xmlEscape(sb, param);
+ sb.append("</string></value></param>");
+ }
+ sb.append("</params></methodCall>");
+ return sb.toString();
+ }
+
+ /**
+ * POST the request body to the ping target and return the response bytes,
+ * capped at {@link #MAX_RESPONSE_BYTES}.
+ */
+ private static byte[] exchange(String pingTargetUrl, String requestBody)
throws IOException {
+ URL url = new URL(pingTargetUrl);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
Review Comment:
🤖Claude: not fixed — by design. Weblog update ping targets are configured by
the global admin, and the project security model (docs/security-model.md)
treats the server making outbound requests to admin-configured Planet feeds and
ping targets as allowed, not a vulnerability. This outbound request is
pre-existing behaviour (the previous XML-RPC client made the same request);
this PR does not widen it, and adds a connection/read timeout and a
response-size cap it previously lacked. Leaving the CodeQL alert for a
maintainer to dismiss as by-design.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
