Copilot commented on code in PR #132:
URL: https://github.com/apache/maven-doap-plugin/pull/132#discussion_r2616420359
##########
src/main/java/org/apache/maven/plugin/doap/DoapMojo.java:
##########
@@ -1348,12 +1348,6 @@ private void writeReleases(XMLWriter writer,
MavenProject project) throws MojoEx
}
String fileRelease = repo.getUrl() + "/" +
repo.pathOf(artifactRelease);
- try {
- DoapUtil.fetchURL(settings, new URL(fileRelease));
- } catch (IOException e) {
- getLog().debug("IOException :" + e.getMessage());
- continue;
- }
DoapUtil.writeElement(writer, doapOptions.getXmlnsPrefix(),
"file-release", fileRelease);
Review Comment:
Removing the URL validation changes the behavior of this method. Previously,
the file-release element was only written if the URL was successfully fetched
(validated). Now, the file-release element is always written even if the URL
doesn't exist or is inaccessible. This could result in DOAP files containing
invalid or non-existent file-release URLs. Either restore the validation logic
using a different approach (e.g., java.net.HttpURLConnection) or document this
as an intentional behavior change.
##########
src/main/java/org/apache/maven/plugin/doap/DoapUtil.java:
##########
@@ -458,77 +444,22 @@ public static boolean isValidEmail(String str) {
}
/**
- * Fetch an URL
+ * Pings a URL.
Review Comment:
The Javadoc comment "Pings a URL" is misleading. The new implementation
doesn't truly ping the URL - it just attempts to open a stream and immediately
closes it. Consider updating the comment to be more accurate about what this
deprecated method actually does, such as "Attempts to open a connection to a
URL."
```suggestion
* Attempts to open a connection to a URL.
```
##########
src/main/java/org/apache/maven/plugin/doap/DoapUtil.java:
##########
@@ -458,77 +444,22 @@ public static boolean isValidEmail(String str) {
}
/**
- * Fetch an URL
+ * Pings a URL.
*
- * @param settings the user settings used to fetch the url with an active
proxy, if defined.
+ * @param settings ignored
* @param url the url to fetch
* @throws IOException if any
- * @see #DEFAULT_TIMEOUT
* @since 1.1
+ * @deprecated use a different library to load a URL.
*/
+ @Deprecated
+ @SuppressWarnings("checkstyle:emptyblock")
public static void fetchURL(Settings settings, URL url) throws IOException
{
if (url == null) {
throw new IllegalArgumentException("The url is null");
}
- if ("file".equals(url.getProtocol())) {
- InputStream in = null;
- try {
- in = url.openStream();
- in.close();
- in = null;
- } finally {
- IOUtil.close(in);
- }
-
- return;
- }
-
- // http, https...
- HttpClient httpClient = new HttpClient(new
MultiThreadedHttpConnectionManager());
-
httpClient.getHttpConnectionManager().getParams().setConnectionTimeout(DEFAULT_TIMEOUT);
-
httpClient.getHttpConnectionManager().getParams().setSoTimeout(DEFAULT_TIMEOUT);
-
httpClient.getParams().setBooleanParameter(HttpClientParams.ALLOW_CIRCULAR_REDIRECTS,
true);
-
- // Some web servers don't allow the default user-agent sent by
httpClient
- httpClient
- .getParams()
- .setParameter(HttpMethodParams.USER_AGENT, "Mozilla/4.0
(compatible; MSIE 6.0; Windows NT 5.0)");
-
- if (settings != null && settings.getActiveProxy() != null) {
- Proxy activeProxy = settings.getActiveProxy();
-
- ProxyInfo proxyInfo = new ProxyInfo();
- proxyInfo.setNonProxyHosts(activeProxy.getNonProxyHosts());
-
- if (StringUtils.isNotEmpty(activeProxy.getHost())
- && !ProxyUtils.validateNonProxyHosts(proxyInfo,
url.getHost())) {
-
httpClient.getHostConfiguration().setProxy(activeProxy.getHost(),
activeProxy.getPort());
-
- if (StringUtils.isNotEmpty(activeProxy.getUsername()) &&
activeProxy.getPassword() != null) {
- Credentials credentials =
- new
UsernamePasswordCredentials(activeProxy.getUsername(),
activeProxy.getPassword());
-
- httpClient.getState().setProxyCredentials(AuthScope.ANY,
credentials);
- }
- }
- }
-
- GetMethod getMethod = new GetMethod(url.toString());
- try {
- int status;
- try {
- status = httpClient.executeMethod(getMethod);
- } catch (SocketTimeoutException e) {
- // could be a sporadic failure, one more retry before we give
up
- status = httpClient.executeMethod(getMethod);
- }
-
- if (status != HttpStatus.SC_OK) {
- throw new FileNotFoundException(url.toString());
- }
- } finally {
- getMethod.releaseConnection();
+ try (InputStream in = url.openStream()) {
Review Comment:
The empty try-with-resources block is unclear and may confuse readers. While
it does test if the URL can be opened, the empty block makes the intent
non-obvious. Consider adding a comment inside the block to clarify that the
method is intentionally just testing URL accessibility, or simplify by removing
the method body entirely since the method is deprecated anyway.
```suggestion
try (InputStream in = url.openStream()) {
// Intentionally left empty: just testing if the URL can be
opened (i.e., is accessible)
```
--
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]
