Skip to content

Commit

Permalink
#343 Fix misc. SonarLint issues (scout rule)
Browse files Browse the repository at this point in the history
  • Loading branch information
ascheman committed Sep 18, 2024
1 parent db1f7db commit 3b1ddf1
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 111 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.aim42.htmlsanitycheck;

import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;
Expand Down Expand Up @@ -32,11 +33,18 @@ public class Configuration {
File sourceDir;
File checkingResultsDir;
File junitResultsDir;
Boolean consoleReport;
Boolean failOnErrors;
Integer httpConnectionTimeout;
Boolean ignoreLocalhost;
Boolean ignoreIPAddresses;
@Builder.Default
Boolean consoleReport = false;
@Builder.Default
Boolean failOnErrors = false;
@Builder.Default
Integer httpConnectionTimeout = 5000;
@Getter(AccessLevel.NONE)
@Builder.Default
Boolean ignoreLocalhost = false;
@Getter(AccessLevel.NONE)
@Builder.Default
Boolean ignoreIPAddresses = false;
/*
* Explanation for configuring http status codes:
* The standard http status codes are defined in class @link NetUtil and can
Expand Down Expand Up @@ -155,4 +163,12 @@ public void validate() throws MisconfigurationException {
throw new MisconfigurationException("checks to execute have to be a non-empty list");
}
}

public boolean isIgnoreLocalhost() {
return ignoreLocalhost != null && ignoreLocalhost;
}

public boolean isIgnoreIPAddresses() {
return ignoreIPAddresses != null && ignoreIPAddresses;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
@Slf4j
class BrokenHttpLinksChecker extends Checker {

static {
TrustAllCertificates.install();
}

// get the (configured) statusCodes, just syntactic sugar...
private final Set<Integer> successCodes;
private final Set<Integer> warningCodes;
Expand Down Expand Up @@ -97,92 +101,85 @@ private void checkAllHttpLinks() {


protected void doubleCheckSingleHttpLink(String href) {


// to create appropriate error messages
String problem;

// bookkeeping:
getCheckingResults().incNrOfChecks();

try {
URL url = new URL(href);

// check if localhost-URL
checkIfLocalhostURL(url, href);

// check if (numerical) IP address
checkIfIPAddress(url, href);
checkHttpLinkWithRetry(url, href);
} catch (MalformedURLException exception) {
Finding malformedURLFinding = new Finding("malformed URL exception with href=" + href);
getCheckingResults().addFinding(malformedURLFinding);
}
}

private void checkHttpLinkWithRetry(URL url, String href) {
String problem;
try {
HttpURLConnection firstConnection = getNewURLConnection(url);

try {
HttpURLConnection firstConnection = getNewURLConnection(url);
// try to connect
firstConnection.connect();
int responseCode = firstConnection.getResponseCode();

// try to connect
firstConnection.connect();
int responseCode = firstConnection.getResponseCode();
// issue 218 and 219: some web servers respond with 403 or 405
// when given HEAD requests. Therefore, try to GET
if (successCodes.contains(responseCode)) {
return;
}
// issue 244: special case for redirects
// thanks to https://stackoverflow.com/questions/39718059/read-from-url-in-groovy-with-redirect
else if (Web.HTTP_REDIRECT_CODES.contains(responseCode)) {
String newLocation;
if (firstConnection.getHeaderField("Location") != null) {
newLocation = firstConnection.getHeaderField("Location");

problem = String.format("Warning: %s returned statuscode %d, new location: %s", href, responseCode, newLocation);
getCheckingResults().addFinding(new Finding(problem));

// issue 218 and 219: some web servers respond with 403 or 405
// when given HEAD requests. Therefore, try GET
if (successCodes.contains(responseCode)) {
}
}
// in case of errors or warnings,
// try again with GET.
else {
HttpURLConnection secondConnection = getNewURLConnection(url);
secondConnection.setRequestMethod("GET");
int finalResponseCode = secondConnection.getResponseCode();
secondConnection.disconnect();

if (successCodes.contains(finalResponseCode)) {
return;
} else if (warningCodes.contains(finalResponseCode)) {
problem = "Warning:";
} else if (errorCodes.contains(finalResponseCode)) {
problem = "Error:";
} else {
problem = "Error: Unknown or unclassified response code:";
}
// issue 244: special case for redirects
// thanks to https://stackoverflow.com/questions/39718059/read-from-url-in-groovy-with-redirect
else if (Web.HTTP_REDIRECT_CODES.contains(responseCode)) {
String newLocation;
if (firstConnection.getHeaderField("Location") != null) {
newLocation = firstConnection.getHeaderField("Location");

problem = String.format("Warning: %s returned statuscode %d, new location: %s", href, responseCode, newLocation);
getCheckingResults().addFinding(new Finding(problem));
problem += String.format(" %s returned statuscode %d.", href, responseCode);

}
}
// in case of errors or warnings,
// try again with GET.
else {
HttpURLConnection secondConnection = getNewURLConnection(url);
secondConnection.setRequestMethod("GET");
int finalResponseCode = secondConnection.getResponseCode();
secondConnection.disconnect();

if (successCodes.contains(finalResponseCode)) {
return;
} else if (warningCodes.contains(finalResponseCode)) {
problem = "Warning:";
} else if (errorCodes.contains(finalResponseCode)) {
problem = "Error:";
} else {
problem = "Error: Unknown or unclassified response code:";
}

problem += String.format(" %s returned statuscode %d.", href, responseCode);
getCheckingResults().addFinding(new Finding(problem));

getCheckingResults().addFinding(new Finding(problem));
} // else

} // else
// cleanup firstConnection
firstConnection.disconnect();

// cleanup firstConnection
firstConnection.disconnect();

} catch (UnknownHostException exception) {
Finding unknownHostFinding = new Finding("Unknown host with href=" + href);
getCheckingResults().addFinding(unknownHostFinding);
} catch (IOException exception) {
Finding someException = new Finding("exception " + exception + " with href=" + href);
getCheckingResults().addFinding(someException);
}
} catch (MalformedURLException exception) {
Finding malformedURLFinding = new Finding("malformed URL exception with href=" + href);
getCheckingResults().addFinding(malformedURLFinding);
} catch (UnknownHostException exception) {
Finding unknownHostFinding = new Finding("Unknown host with href=" + href);
getCheckingResults().addFinding(unknownHostFinding);
} catch (IOException exception) {
Finding someException = new Finding("exception " + exception + " with href=" + href);
getCheckingResults().addFinding(someException);
}
}


private HttpURLConnection getNewURLConnection(URL url) throws IOException {

TrustAllCertificates.install();

HttpURLConnection connection = (HttpURLConnection) url.openConnection();
connection.setRequestMethod("HEAD");

Expand All @@ -205,7 +202,7 @@ private HttpURLConnection getNewURLConnection(URL url) throws IOException {

// if configured, ip addresses in URLs yield warnings
private void checkIfIPAddress(URL url, String href) {
if (!getMyConfig().getIgnoreIPAddresses()) {
if (!getMyConfig().isIgnoreIPAddresses()) {
String host = url.getHost();

if (host.matches("\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}")) {
Expand All @@ -217,7 +214,7 @@ private void checkIfIPAddress(URL url, String href) {

// if configured ,localhost-URLs yield warnings!
private void checkIfLocalhostURL(URL url, String href) {
if (!getMyConfig().getIgnoreLocalhost()) {
if (!getMyConfig().isIgnoreLocalhost()) {
String host = url.getHost();
if (("localhost".equals(host)) || host.startsWith("127.0.0")) {
Finding localhostWarning = new Finding("Warning: localhost urls indicates suspicious environment dependency: href=" + href);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

@Slf4j
public class DuplicateIdChecker extends Checker {
private Set<String> idStringsSet;
private List<String> idStringsList;

public DuplicateIdChecker(Configuration pConfig) {
Expand All @@ -27,6 +26,7 @@ protected void initCheckingResultsDescription() {

@Override
protected SingleCheckResults check(final HtmlPage pageToCheck) {
Set<String> idStringsSet;
log.trace("Checking '{}'", pageToCheck.getFile());

//get list of all tagsWithId '<... id="XYZ"...' in html file
Expand Down Expand Up @@ -55,20 +55,4 @@ private void checkForDuplicateDefinition(final String idString) {
}

}

public Set<String> getIdStringsSet() {
return idStringsSet;
}

public void setIdStringsSet(Set<String> idStringsSet) {
this.idStringsSet = idStringsSet;
}

public List<String> getIdStringsList() {
return idStringsList;
}

public void setIdStringsList(List<String> idStringsList) {
this.idStringsList = idStringsList;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ private void checkDuplicateMapNames() {
Set<String> mapNameSet = new HashSet<>(mapNames);

mapNameSet.stream()
.peek(a -> getCheckingResults().incNrOfChecks())
.filter(name -> mapNames.stream().filter(name2 -> name2.equals(name)).count() > 1)
.filter(name -> {
getCheckingResults().incNrOfChecks();
return mapNames.stream().filter(name2 -> name2.equals(name)).count() > 1;
})
.forEach(mapName ->
getCheckingResults().addFinding(
new Finding(mapNames.stream().filter(name2 -> name2.equals(mapName)).count() + " imagemaps with identical name \"" + mapName + "\" exist.")));
Expand Down Expand Up @@ -138,8 +140,10 @@ private void checkAreaHrefsForMapName(String mapName) {
// TODO replace checkEmptyMaps with additional check here

areaHrefs.stream()
.peek(a -> getCheckingResults().incNrOfChecks())
.filter(Web::isCrossReference)
.filter(href -> {
getCheckingResults().incNrOfChecks();
return Web.isCrossReference(href);
})
.forEach(href -> checkLocalHref(href, mapName, areaHrefs));

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ protected SingleCheckResults check(final HtmlPage pageToCheck) {
getCheckingResults().setNrOfChecks(pageToCheck.getAllImageTags().size());

// see HtmlPageSpec for behavior: missing or empty alt-attributes are included...
pageToCheck.getAllImageTagsWithMissingAltAttribute().stream()
.forEach(element -> reportSingleImageTagWithMissingAlt(element));
pageToCheck.getAllImageTagsWithMissingAltAttribute()
.forEach(this::reportSingleImageTagWithMissingAlt);


return getCheckingResults();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected SingleCheckResults check(final HtmlPage pageToCheck) {
//get list of all image-tags "<img..." in html file
images = pageToCheck.getAllImageTags();

logger.debug("images to check: " + images);
logger.debug("images to check: {}", images);

checkAllImages();

Expand All @@ -67,17 +67,16 @@ private void checkSingleLocalImage(HtmlElement image) {
boolean isRemoteURL = Web.isRemoteURL(imageSrcAttribute);
boolean isDataURI = Web.isDataURI(imageSrcAttribute);
if (!isRemoteURL) {
getCheckingResults().incNrOfChecks();
if (isDataURI) {
// bookkeeping:
getCheckingResults().incNrOfChecks();

doesDataURIContainData(imageSrcAttribute);

} else {
//we have a simple local image

// bookkeeping:
getCheckingResults().incNrOfChecks();

doesImageFileExist(imageSrcAttribute);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.aim42.htmlsanitycheck.Configuration;
import org.aim42.htmlsanitycheck.collect.SingleCheckResults;
import org.aim42.htmlsanitycheck.html.HtmlPage;
import org.aim42.htmlsanitycheck.tools.InvalidUriSyntaxException;
import org.aim42.htmlsanitycheck.tools.Web;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -61,7 +62,7 @@ protected SingleCheckResults check(final HtmlPage pageToCheck) {
// created from the List of all by toSet() method
Set<String> localResourcesSet = new HashSet<>(localResourcesList);

logger.debug("local resources set: " + localResourcesSet);
logger.debug("local resources set: {}", localResourcesSet);

final File file1 = pageToCheck.getFile();
final File file = (file1 == null ? null : file1.getParentFile());
Expand All @@ -82,7 +83,7 @@ private void checkAllLocalResources(Set<String> localResources) {
private void checkSingleLocalResource(String localResource) {
// the localResource is either path+filename or filename or directory

logger.debug("single resource to be checked: + " + localResource);
logger.debug("single resource to be checked: {}", localResource);

// bookkeeping:
getCheckingResults().incNrOfChecks();
Expand All @@ -92,13 +93,12 @@ private void checkSingleLocalResource(String localResource) {
try {
localResourcePath = new URI(localResource).getPath();
} catch (URISyntaxException e) {
throw new RuntimeException(e);
throw new InvalidUriSyntaxException(e);
}

if (localResourcePath == null) {
// For example, javascript:;
return;

}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,4 @@ public class UnknownCheckerException extends RuntimeException {
public UnknownCheckerException(String message) {
super(message);
}

public UnknownCheckerException(String message, String checkerName) {
super(message + ": " + checkerName);
}
}
Loading

0 comments on commit 3b1ddf1

Please sign in to comment.