[SPARK-52792][CORE] Remove commons-lang3 dependency from network-common

[dongjoon-hyun]

What changes were proposed in this pull request?

This PR aims to remove commons-lang3 dependency from network-common module.

Why are the changes needed?

To make network-common module independent from the 3rd-party dependency and its vulnerability.

BEFORE

$ mvn dependency:tree -pl common/network-common | grep commons-lang3
[INFO] +- org.apache.commons:commons-lang3:jar:3.18.0:compile

AFTER

$ mvn dependency:tree -pl common/network-common | grep commons-lang3

Does this PR introduce any user-facing change?

No behavior change.

How was this patch tested?

Pass the CIs.

Was this patch authored or co-authored using generative AI tooling?

No.

[dongjoon-hyun]

This is the equivalent of the original.

https://github.com/apache/commons-lang/blame/2a513149dd92dd8aa57f8fbfae867ca044755565/src/main/java/org/apache/commons/lang3/SystemUtils.java#L1646

public static final boolean IS_OS_WINDOWS = getOsMatchesName(OS_NAME_WINDOWS_PREFIX);

https://github.com/apache/commons-lang/blob/4711998927a52abe70bec7075f9f89e02824b404/src/main/java/org/apache/commons/lang3/SystemUtils.java#L2160-L2162

return isOsNameMatch(OS_NAME, osNamePrefix);

return Strings.CI.startsWith(osName, osNamePrefix);

return CharSequenceUtils.regionMatches(str, ignoreCase, 0, prefix, 0, preLen);

return ((String) cs).regionMatches(ignoreCase, thisStart, (String) substring, start, length);

https://github.com/apache/commons-lang/blob/4711998927a52abe70bec7075f9f89e02824b404/src/main/java/org/apache/commons/lang3/SystemUtils.java#L597

public static final String OS_NAME = SystemProperties.getOsName();

return getProperty(OS_NAME);

public static final String OS_NAME = "os.name";

[viirya]

I confirmed that OS_NAME_WINDOWS_PREFIX is "Windows".

// SystemUtils.java
private static boolean getOsMatchesName(final String osNamePrefix) {
  return isOsNameMatch(OS_NAME, osNamePrefix);
}

static boolean isOsNameMatch(final String osName, final String osNamePrefix) {
  if (osName == null) {
    return false;
  }
  return osName.startsWith(osNamePrefix);
}

public static final String OS_NAME = SystemProperties.getOsName();

// SystemProperties.java
public static String getOsName() {
  return getProperty(OS_NAME);
}
public static final String OS_NAME = "os.name";

[dongjoon-hyun]

Could you review this PR, @viirya ?

[dongjoon-hyun]

Thank you for reviewing. It's updated like the following. 7 is preLen.

String name = System.getProperty("os.name");
boolean isNotWindows = 7 > name.length() || !name.regionMatches(true, 0, "Windows", 0, 7);

[viirya]

Pending CI

[dongjoon-hyun]

Thank you so much, @viirya !

[peter-toth]

Late LGTM.

[mridulm]

We dont need the 7 > name.length() || here

[dongjoon-hyun]

It's originated from the previous commons-lang3 code. Initially, I also thought like you. However, after Liang-Chi commented, I realized that we need it in the following case, @mridulm .

If System.getProperty("os.name") == Win, 7 > name.length() will return true here. It means isNotWindows = true. Otherwise, isNotWindows becomes false for Win.

jshell> String name = "Win"
name ==> "Win"

jshell> !name.regionMatches(true, 0, "Windows", 0, 1)
$3 ==> false

[mridulm]

scala> val name = "Win"
name: String = Win

scala> !name.regionMatches(true, 0, "Windows", 0, 7)
res2: Boolean = true

scala> 

It should be 0, 7), no ?

[viirya]

Hmm, the length check comes from commons-lang3's startsWith.

I recommended to also add the check as I thought that regionMatches will throw StringIndexOutOfBoundsException if the length of str is less than preLen`, based on search answer from Google.

But seems in @mridulm's test above, it doesn't. The source code of regionMatches can confirm it.

public boolean regionMatches(int toffset, String other, int ooffset, int len) {
        // Note: toffset, ooffset, or len might be near -1>>>1.
        if ((ooffset < 0) || (toffset < 0) ||
             (toffset > (long)length() - len) ||
             (ooffset > (long)other.length() - len)) {
            return false;
        }
...
}

[dongjoon-hyun]

Oh, got it. It seems that I made a mistake yesterday during testing.

It should be 0, 7), no ?

Thank you, @mridulm and @viirya . Let me do the follow-up.

[dongjoon-hyun]

Here is a follow-up.

• [SPARK-52792][CORE][FOLLOWUP] Remove redundant condition check #51524