On 18/01/2021 07:29, Konstantin Kolinko wrote: > пт, 15 янв. 2021 г. в 18:21, <ma...@apache.org>: >> >> This is an automated email from the ASF dual-hosted git repository. >> >> markt pushed a commit to branch 9.0.x >> in repository https://gitbox.apache.org/repos/asf/tomcat.git >> >> commit 9eaf35f18b3e6dbd1e0920a3640538aa765d860d >> Author: Mark Thomas <ma...@apache.org> >> AuthorDate: Fri Jan 15 15:07:13 2021 +0000 >> >> Align with Codec >> --- >> java/org/apache/tomcat/util/codec/binary/Base64.java | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/java/org/apache/tomcat/util/codec/binary/Base64.java >> b/java/org/apache/tomcat/util/codec/binary/Base64.java >> index 08d7d34..3cd4f1a 100644 >> --- a/java/org/apache/tomcat/util/codec/binary/Base64.java >> +++ b/java/org/apache/tomcat/util/codec/binary/Base64.java >> @@ -17,6 +17,7 @@ >> package org.apache.tomcat.util.codec.binary; >> >> import java.math.BigInteger; >> +import java.util.Objects; >> >> /** >> * Provides Base64 encoding and decoding as defined by <a >> href="http://www.ietf.org/rfc/rfc2045.txt";>RFC 2045</a>. >> @@ -331,9 +332,7 @@ public class Base64 extends BaseNCodec { >> * @since 1.4 >> */ >> public static byte[] encodeInteger(final BigInteger bigInteger) { >> - if (bigInteger == null) { >> - throw new >> NullPointerException(sm.getString("base64.nullEncodeParameter")); >> - } >> + >> Objects.requireNonNull(bigInteger,sm.getString("base64.nullEncodeParameter")); > > I think it is better to revert this change, or remove this (unused) > method "encodeInteger" altogether. > > This change degrades performance, as sm.getString() is called unconditionally. > Original code in Apache Commons Codec uses String as the second parameter, > > https://github.com/apache/commons-codec/blob/master/src/main/java/org/apache/commons/codec/binary/Base64.java#L318 > > There exists a method Objects.requireNonNull(T, Supplier<String>), but > whether creating a lambda creates less overhead than calling > sm.getString() is something that I have not tested.
I'm on the fence. I agree the new version is less efficient but if the code is never used does that really matter? That then opens up the question of whether or not the unused code should be removed. For the codec port, the original aim was to keep the code the same as the original apart from package name. The idea being that that would make keeping the port and the original in sync easier. The original and the port have diverged a little over time which undermines the "keep them the same" argument a little. If you want to revert this change, I'm not going to object. If this method is removed, we should probably remove all the unused code in the codec port. Something to consider for 10.1.x? Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
