Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further reduce usages of StringUtils #9044

Merged
merged 13 commits into from
Mar 27, 2024
3 changes: 1 addition & 2 deletions cli/src/main/java/hudson/cli/CLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import org.apache.commons.lang.StringUtils;
import org.glassfish.tyrus.client.ClientManager;
import org.glassfish.tyrus.client.ClientProperties;
import org.glassfish.tyrus.client.SslEngineConfigurator;
Expand Down Expand Up @@ -246,7 +245,7 @@ public static int _main(String[] _args) throws Exception {
if (auth == null && bearer == null) {
// -auth option not set
if ((userIdEnv != null && !userIdEnv.isBlank()) && (tokenEnv != null && !tokenEnv.isBlank())) {
auth = StringUtils.defaultString(userIdEnv).concat(":").concat(StringUtils.defaultString(tokenEnv));
auth = userIdEnv.concat(":").concat(tokenEnv);
BobDu marked this conversation as resolved.
Show resolved Hide resolved
} else if ((userIdEnv != null && !userIdEnv.isBlank()) || (tokenEnv != null && !tokenEnv.isBlank())) {
printUsage(Messages.CLI_BadAuth());
return -1;
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/Functions.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@
import org.apache.commons.jelly.XMLOutput;
import org.apache.commons.jexl.parser.ASTSizeFunction;
import org.apache.commons.jexl.util.Introspector;
import org.apache.commons.lang.StringUtils;
import org.jenkins.ui.icon.Icon;
import org.jenkins.ui.icon.IconSet;
import org.jvnet.tiger_types.Types;
Expand Down Expand Up @@ -2415,7 +2414,7 @@ public static Icon tryGetIcon(String iconGuess) {
}

private static @NonNull String filterIconNameClasses(@NonNull String classNames) {
return Arrays.stream(StringUtils.split(classNames, ' '))
return Arrays.stream(classNames.split(" "))
.filter(className -> className.startsWith("icon-"))
.collect(Collectors.joining(" "));
}
Expand Down
11 changes: 5 additions & 6 deletions core/src/main/java/hudson/PluginManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.LogFactory;
import org.jenkinsci.Symbol;
import org.jvnet.hudson.reactor.Executable;
Expand Down Expand Up @@ -1436,13 +1435,13 @@
if (query == null || query.isBlank()) {
return true;
}
return StringUtils.containsIgnoreCase(plugin.name, query) ||
StringUtils.containsIgnoreCase(plugin.title, query) ||
StringUtils.containsIgnoreCase(plugin.excerpt, query) ||
return (plugin.name != null && plugin.name.toLowerCase().contains(query.toLowerCase())) ||

Check warning on line 1438 in core/src/main/java/hudson/PluginManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1438 is only partially covered, 2 branches are missing
(plugin.title != null && plugin.title.toLowerCase().contains(query.toLowerCase())) ||

Check warning on line 1439 in core/src/main/java/hudson/PluginManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1439 is only partially covered, 2 branches are missing
(plugin.excerpt != null && plugin.excerpt.toLowerCase().contains(query.toLowerCase())) ||

Check warning on line 1440 in core/src/main/java/hudson/PluginManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1440 is only partially covered, one branch is missing
plugin.hasCategory(query) ||
plugin.getCategoriesStream()
.map(UpdateCenter::getCategoryDisplayName)
.anyMatch(category -> StringUtils.containsIgnoreCase(category, query)) ||
.anyMatch(category -> category != null && category.toLowerCase().contains(query.toLowerCase())) ||

Check warning on line 1444 in core/src/main/java/hudson/PluginManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1444 is only partially covered, 3 branches are missing
plugin.hasWarnings() && query.equalsIgnoreCase("warning:");
})
.limit(Math.max(limit - plugins.size(), 1))
Expand Down Expand Up @@ -1472,7 +1471,7 @@
jsonObject.put("title", plugin.title);
jsonObject.put("displayName", plugin.getDisplayName());
if (plugin.wiki == null || !(plugin.wiki.startsWith("https://") || plugin.wiki.startsWith("http://"))) {
jsonObject.put("wiki", StringUtils.EMPTY);
jsonObject.put("wiki", "");
} else {
jsonObject.put("wiki", plugin.wiki);
}
Expand Down
8 changes: 6 additions & 2 deletions core/src/main/java/hudson/PluginWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
import jenkins.plugins.DetachedPluginsUtil;
import jenkins.security.UpdateSiteWarningsMonitor;
import jenkins.util.URLClassLoader2;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.LogFactory;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.Beta;
Expand Down Expand Up @@ -497,7 +496,12 @@

@Override
public String getDisplayName() {
return StringUtils.removeStart(getLongName(), "Jenkins ");
String displayName = getLongName();
String removePrefix = "Jenkins ";
if (displayName != null && displayName.startsWith(removePrefix)) {

Check warning on line 501 in core/src/main/java/hudson/PluginWrapper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 501 is only partially covered, one branch is missing
return displayName.substring(removePrefix.length());
}
return displayName;
}

public Api getApi() {
Expand Down
13 changes: 11 additions & 2 deletions core/src/main/java/hudson/cli/CLIAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import jenkins.util.SystemProperties;
import jenkins.websocket.WebSocketSession;
import jenkins.websocket.WebSockets;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -128,7 +127,17 @@
}
if (ALLOW_WEBSOCKET == null) {
final String actualOrigin = req.getHeader("Origin");
final String expectedOrigin = StringUtils.removeEnd(StringUtils.removeEnd(Jenkins.get().getRootUrlFromRequest(), "/"), req.getContextPath());

String o = Jenkins.get().getRootUrlFromRequest();
String removeSuffix1 = "/";
if (o.endsWith(removeSuffix1)) {

Check warning on line 133 in core/src/main/java/hudson/cli/CLIAction.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 133 is only partially covered, one branch is missing
o = o.substring(0, o.length() - removeSuffix1.length());
}
String removeSuffix2 = req.getContextPath();
if (o.endsWith(removeSuffix2)) {

Check warning on line 137 in core/src/main/java/hudson/cli/CLIAction.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 137 is only partially covered, one branch is missing
o = o.substring(0, o.length() - removeSuffix2.length());
}
final String expectedOrigin = o;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extra o variable seems like an unnecessary layer of indirection for the reader. Why not just name it expectedOrigin to begin with and remove this line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expectedOrigin is a local variable used in lambda, it must be a final or effectively final.

Of course we can set o as another name for readability, but I think it is not very necessary as a temporary variable.


if (actualOrigin == null || !actualOrigin.equals(expectedOrigin)) {
LOGGER.log(Level.FINE, () -> "Rejecting origin: " + actualOrigin + "; expected was from request: " + expectedOrigin);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.util.List;
import javax.servlet.ServletException;
import jenkins.model.Jenkins;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
Expand Down Expand Up @@ -173,6 +172,6 @@
}

private static boolean startsWithImpl(String str, String prefix, boolean ignoreCase) {
return ignoreCase ? StringUtils.startsWithIgnoreCase(str, prefix) : str.startsWith(prefix);
return ignoreCase ? str.toLowerCase().startsWith(prefix.toLowerCase()) : str.startsWith(prefix);

Check warning on line 175 in core/src/main/java/hudson/model/AutoCompletionCandidates.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 175 is only partially covered, one branch is missing
basil marked this conversation as resolved.
Show resolved Hide resolved
}
}
5 changes: 2 additions & 3 deletions core/src/main/java/hudson/model/Fingerprint.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
import jenkins.model.FingerprintFacet;
import jenkins.model.Jenkins;
import jenkins.model.TransientFingerprintFacetFactory;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;
import org.springframework.security.access.AccessDeniedException;
Expand Down Expand Up @@ -681,7 +680,7 @@ public static RangeSet fromString(String list, boolean skipError) {
}

String[] items = Util.tokenize(list, ",");
if (items.length > 1 && items.length <= StringUtils.countMatches(list, ",")) {
if (items.length > 1 && items.length <= list.chars().filter(c -> c == ',').count()) {
if (!skipError) {
throw new IllegalArgumentException(
String.format("Unable to parse '%s', expected correct notation M,N or M-N", list));
Expand All @@ -704,7 +703,7 @@ public static RangeSet fromString(String list, boolean skipError) {
}

if (s.contains("-")) {
if (StringUtils.countMatches(s, "-") > 1) {
if (s.chars().filter(c -> c == '-').count() > 1) {
if (!skipError) {
throw new IllegalArgumentException(String.format(
"Unable to parse '%s', expected correct notation M,N or M-N", list));
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/model/Slave.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.jar.JarFile;
import java.util.jar.Manifest;
Expand All @@ -73,7 +74,6 @@
import jenkins.security.MasterToSlaveCallable;
import jenkins.slaves.WorkspaceLocator;
import jenkins.util.SystemProperties;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -393,7 +393,7 @@ public FilePath getRootPath() {
// if computer is null then channel is null and thus we were going to return null anyway
return null;
} else {
return createPath(StringUtils.defaultString(computer.getAbsoluteRemoteFs(), remoteFS));
return createPath(Objects.toString(computer.getAbsoluteRemoteFs(), remoteFS));
}
}

Expand Down
7 changes: 5 additions & 2 deletions core/src/main/java/hudson/model/UpdateSite.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
import net.sf.json.JSONArray;
import net.sf.json.JSONException;
import net.sf.json.JSONObject;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -1299,7 +1298,11 @@
displayName = title;
else
displayName = name;
return StringUtils.removeStart(displayName, "Jenkins ");
String removePrefix = "Jenkins ";
if (displayName != null && displayName.startsWith(removePrefix)) {

Check warning on line 1302 in core/src/main/java/hudson/model/UpdateSite.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1302 is only partially covered, 2 branches are missing
return displayName.substring(removePrefix.length());

Check warning on line 1303 in core/src/main/java/hudson/model/UpdateSite.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 1303 is not covered by tests
}
return displayName;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/search/FixedSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import org.apache.commons.lang.StringUtils;

/**
* Set of {@link SearchItem}s that are statically known upfront.
Expand Down Expand Up @@ -62,7 +61,7 @@
boolean caseInsensitive = UserSearchProperty.isCaseInsensitive();
for (SearchItem i : items) {
String name = i.getSearchName();
if (name != null && (name.contains(token) || (caseInsensitive && StringUtils.containsIgnoreCase(name, token)))) {
if (name != null && (name.contains(token) || (caseInsensitive && name.toLowerCase().contains(token.toLowerCase())))) {

Check warning on line 64 in core/src/main/java/hudson/search/FixedSet.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 64 is only partially covered, 3 branches are missing
basil marked this conversation as resolved.
Show resolved Hide resolved
result.add(i);
}
}
Expand Down
6 changes: 4 additions & 2 deletions core/src/main/java/hudson/security/SecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
import jenkins.security.BasicHeaderProcessor;
import jenkins.util.SystemProperties;
import net.sf.json.JSONObject;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
Expand Down Expand Up @@ -672,7 +671,10 @@

// If deduced entry point isn't deduced yet or the content is a blank value
// use the root web point "/" as a fallback
from = StringUtils.defaultIfBlank(from, "/").trim();
if (from == null || from.isBlank()) {

Check warning on line 674 in core/src/main/java/hudson/security/SecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 674 is only partially covered, 2 branches are missing
from = "/";

Check warning on line 675 in core/src/main/java/hudson/security/SecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 675 is not covered by tests
}
from = from.trim();

// Encode the return value
String returnValue = URLEncoder.encode(from, StandardCharsets.UTF_8);
Expand Down
9 changes: 4 additions & 5 deletions core/src/main/java/hudson/slaves/JNLPLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import jenkins.slaves.RemotingWorkDirSettings;
import jenkins.util.SystemProperties;
import jenkins.websocket.WebSockets;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
Expand Down Expand Up @@ -229,8 +228,8 @@
* {@link Jenkins#checkGoodName(String)} saves us from most troublesome characters, but we still have to deal with
* spaces and therefore with double quotes and backticks.
*/
private static String escapeUnix(String input) {
if (StringUtils.isAlphanumeric(input)) {
private static String escapeUnix(@NonNull String input) {
if (!input.isEmpty() && input.chars().allMatch(Character::isLetterOrDigit)) {

Check warning on line 232 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 232 is only partially covered, 2 branches are missing
return input;
}
Escaper escaper =
Expand All @@ -242,8 +241,8 @@
* {@link Jenkins#checkGoodName(String)} saves us from most troublesome characters, but we still have to deal with
* spaces and therefore with double quotes.
*/
private static String escapeWindows(String input) {
if (StringUtils.isAlphanumeric(input)) {
private static String escapeWindows(@NonNull String input) {
if (!input.isEmpty() && input.chars().allMatch(Character::isLetterOrDigit)) {

Check warning on line 245 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 245 is only partially covered, 2 branches are missing
return input;
}
Escaper escaper = Escapers.builder().addEscape('"', "\\\"").build();
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/tasks/BuildTrigger.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.util.Collection;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.StringTokenizer;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
Expand All @@ -68,7 +69,6 @@
import jenkins.model.ParameterizedJobMixIn;
import jenkins.triggers.ReverseBuildTrigger;
import net.sf.json.JSONObject;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
Expand Down Expand Up @@ -114,7 +114,7 @@ public BuildTrigger(String childProjects, boolean evenIfUnstable) {

@DataBoundConstructor
public BuildTrigger(String childProjects, String threshold) {
this(childProjects, Result.fromString(StringUtils.defaultString(threshold, Result.SUCCESS.toString())));
this(childProjects, Result.fromString(Objects.toString(threshold, Result.SUCCESS.toString())));
}

public BuildTrigger(String childProjects, Result threshold) {
Expand Down
8 changes: 5 additions & 3 deletions core/src/main/java/jenkins/install/SetupWizard.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package jenkins.install;

import static org.apache.commons.lang.StringUtils.defaultIfBlank;

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
Expand Down Expand Up @@ -486,7 +484,11 @@
File state = getUpdateStateFile();
if (state.exists()) {
try {
from = new VersionNumber(defaultIfBlank(Files.readString(Util.fileToPath(state), StandardCharsets.UTF_8), "1.0").trim());
String version = Files.readString(Util.fileToPath(state), StandardCharsets.UTF_8);
if (version == null || version.isBlank()) {
version = "1.0";
}
from = new VersionNumber(version.trim());

Check warning on line 491 in core/src/main/java/jenkins/install/SetupWizard.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 487-491 are not covered by tests
} catch (IOException ex) {
LOGGER.log(Level.SEVERE, "Cannot read the current version file", ex);
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.springframework.security.authentication.AnonymousAuthenticationToken;
Expand Down Expand Up @@ -65,7 +64,7 @@
HttpServletResponse rsp = (HttpServletResponse) response;
String authorization = req.getHeader("Authorization");

if (StringUtils.startsWithIgnoreCase(authorization, "Basic ")) {
if (authorization != null && authorization.toLowerCase().startsWith("Basic ".toLowerCase())) {

Check warning on line 67 in core/src/main/java/jenkins/security/BasicHeaderProcessor.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 67 is only partially covered, one branch is missing
Copy link
Member

@jglick jglick Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is unlikely to matter in practice, this is not a sound replacement. If a request was sent with

AUTHORIZATION: BASIC czNjcjN0

then a server running in Turkish locale (e.g., #9066, though here it is the server not client locale which matters) would fail this clause because

"basıc cznjcjn0".startsWith("basic ")

is false.

In general, you always want to pass the Locale argument (almost always Locale.ROOT) to toLowerCase / toUpperCase.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BobDu Is it worth filing a follow-up PR to address the above feedback? Or do we think the code is fine as-is?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree always add Locale.ROOT arg to toLowerCase method is a best practices.
Already make a new PR #9162

// authenticate the user
String uidpassword = Scrambler.descramble(authorization.substring(6));
int idx = uidpassword.indexOf(':');
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/org/jenkins/ui/icon/Icon.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.List;
import java.util.Map;
import org.apache.commons.jelly.JellyContext;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

Expand Down Expand Up @@ -238,7 +237,7 @@ public static String toNormalizedIconName(String string) {
if (string == null) {
return null;
}
if (StringUtils.endsWithAny(string, SUPPORTED_FORMATS)) {
if (Arrays.stream(SUPPORTED_FORMATS).anyMatch(string::endsWith)) {
string = string.substring(0, string.length() - 4);
}
return string.replace('_', '-');
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/org/jenkins/ui/symbol/Symbol.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import java.util.logging.Logger;
import jenkins.model.Jenkins;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;

/**
* Helper class to load symbols from Jenkins core or plugins.
Expand Down Expand Up @@ -50,7 +49,7 @@ public static String get(@NonNull SymbolRequest request) {
String pluginName = request.getPluginName();
String id = request.getId();

String identifier = StringUtils.defaultIfBlank(pluginName, "core");
String identifier = (pluginName == null || pluginName.isBlank()) ? "core" : pluginName;

String symbol = SYMBOLS
.computeIfAbsent(identifier, key -> new ConcurrentHashMap<>())
Expand Down
Loading