Enabled NullAway for util package (AsyncHttpClient#1885)

Asapin · web-flow · commit 6f1fb819c669 · 2023-06-05T11:10:31.000+05:30
* Added new failing test when there are 2 duplicating form parameters but one if them has null as a value

* Fixed potentian NPE when form params include two params with the same name and one of the param has null as a value

* Enabled NullAway for util package
diff --git a/client/src/main/java/org/asynchttpclient/proxy/ProxyServer.java b/client/src/main/java/org/asynchttpclient/proxy/ProxyServer.java
@@ -147,7 +147,7 @@ public Builder setSecuredPort(int securedPort) {
             return this;
         }
 
-        public Builder setRealm(Realm realm) {
+        public Builder setRealm(@Nullable Realm realm) {
             this.realm = realm;
             return this;
         }
diff --git a/client/src/main/java/org/asynchttpclient/util/Assertions.java b/client/src/main/java/org/asynchttpclient/util/Assertions.java
@@ -15,20 +15,25 @@
  */
 package org.asynchttpclient.util;
 
+import org.jetbrains.annotations.Contract;
+import org.jetbrains.annotations.Nullable;
+
 public final class Assertions {
 
     private Assertions() {
     }
 
-    public static <T> T assertNotNull(T value, String name) {
+    @Contract(value = "null, _ -> fail", pure = true)
+    public static <T> T assertNotNull(@Nullable T value, String name) {
         if (value == null) {
             throw new NullPointerException(name);
         }
         return value;
 
     }
 
-    public static String assertNotEmpty(String value, String name) {
+    @Contract(value = "null, _ -> fail", pure = true)
+    public static String assertNotEmpty(@Nullable String value, String name) {
         assertNotNull(value, name);
         if (value.length() == 0) {
             throw new IllegalArgumentException("empty " + name);
diff --git a/client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java b/client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java
@@ -19,6 +19,7 @@
 import org.asynchttpclient.spnego.SpnegoEngine;
 import org.asynchttpclient.spnego.SpnegoEngineException;
 import org.asynchttpclient.uri.Uri;
+import org.jetbrains.annotations.Nullable;
 
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
@@ -38,7 +39,7 @@ private AuthenticatorUtils() {
         // Prevent outside initialization
     }
 
-    public static String getHeaderWithPrefix(List<String> authenticateHeaders, String prefix) {
+    public static @Nullable String getHeaderWithPrefix(@Nullable List<String> authenticateHeaders, String prefix) {
         if (authenticateHeaders != null) {
             for (String authenticateHeader : authenticateHeaders) {
                 if (authenticateHeader.regionMatches(true, 0, prefix, 0, prefix.length())) {
@@ -50,11 +51,11 @@ public static String getHeaderWithPrefix(List<String> authenticateHeaders, Strin
         return null;
     }
 
-    private static String computeBasicAuthentication(Realm realm) {
+    private static @Nullable String computeBasicAuthentication(@Nullable Realm realm) {
         return realm != null ? computeBasicAuthentication(realm.getPrincipal(), realm.getPassword(), realm.getCharset()) : null;
     }
 
-    private static String computeBasicAuthentication(String principal, String password, Charset charset) {
+    private static String computeBasicAuthentication(@Nullable String principal, @Nullable String password, Charset charset) {
         String s = principal + ':' + password;
         return "Basic " + Base64.getEncoder().encodeToString(s.getBytes(charset));
     }
@@ -68,9 +69,9 @@ public static String computeRealmURI(Uri uri, boolean useAbsoluteURI, boolean om
         }
     }
 
-    private static String computeDigestAuthentication(Realm realm) {
+    private static String computeDigestAuthentication(Realm realm, Uri uri) {
 
-        String realmUri = computeRealmURI(realm.getUri(), realm.isUseAbsoluteURI(), realm.isOmitQuery());
+        String realmUri = computeRealmURI(uri, realm.isUseAbsoluteURI(), realm.isOmitQuery());
 
         StringBuilder builder = new StringBuilder().append("Digest ");
         append(builder, "username", realm.getPrincipal(), true);
@@ -99,7 +100,7 @@ private static String computeDigestAuthentication(Realm realm) {
         return new String(StringUtils.charSequence2Bytes(builder, ISO_8859_1), StandardCharsets.UTF_8);
     }
 
-    private static void append(StringBuilder builder, String name, String value, boolean quoted) {
+    private static void append(StringBuilder builder, String name, @Nullable String value, boolean quoted) {
         builder.append(name).append('=');
         if (quoted) {
             builder.append('"').append(value).append('"');
@@ -109,7 +110,7 @@ private static void append(StringBuilder builder, String name, String value, boo
         builder.append(", ");
     }
 
-    public static String perConnectionProxyAuthorizationHeader(Request request, Realm proxyRealm) {
+    public static @Nullable String perConnectionProxyAuthorizationHeader(Request request, @Nullable Realm proxyRealm) {
         String proxyAuthorization = null;
         if (proxyRealm != null && proxyRealm.isUsePreemptiveAuth()) {
             switch (proxyRealm.getScheme()) {
@@ -130,7 +131,7 @@ public static String perConnectionProxyAuthorizationHeader(Request request, Real
         return proxyAuthorization;
     }
 
-    public static String perRequestProxyAuthorizationHeader(Request request, Realm proxyRealm) {
+    public static @Nullable String perRequestProxyAuthorizationHeader(Request request, @Nullable Realm proxyRealm) {
         String proxyAuthorization = null;
         if (proxyRealm != null && proxyRealm.isUsePreemptiveAuth()) {
 
@@ -141,11 +142,12 @@ public static String perRequestProxyAuthorizationHeader(Request request, Realm p
                 case DIGEST:
                     if (isNonEmpty(proxyRealm.getNonce())) {
                         // update realm with request information
+                        final Uri uri = request.getUri();
                         proxyRealm = realm(proxyRealm)
-                                .setUri(request.getUri())
+                                .setUri(uri)
                                 .setMethodName(request.getMethod())
                                 .build();
-                        proxyAuthorization = computeDigestAuthentication(proxyRealm);
+                        proxyAuthorization = computeDigestAuthentication(proxyRealm, uri);
                     }
                     break;
                 case NTLM:
@@ -162,7 +164,8 @@ public static String perRequestProxyAuthorizationHeader(Request request, Realm p
         return proxyAuthorization;
     }
 
-    public static String perConnectionAuthorizationHeader(Request request, ProxyServer proxyServer, Realm realm) {
+    public static @Nullable String perConnectionAuthorizationHeader(Request request, @Nullable ProxyServer proxyServer,
+                                                                    @Nullable Realm realm) {
         String authorizationHeader = null;
 
         if (realm != null && realm.isUsePreemptiveAuth()) {
@@ -203,7 +206,7 @@ public static String perConnectionAuthorizationHeader(Request request, ProxyServ
         return authorizationHeader;
     }
 
-    public static String perRequestAuthorizationHeader(Request request, Realm realm) {
+    public static @Nullable String perRequestAuthorizationHeader(Request request, @Nullable Realm realm) {
         String authorizationHeader = null;
         if (realm != null && realm.isUsePreemptiveAuth()) {
 
@@ -214,11 +217,12 @@ public static String perRequestAuthorizationHeader(Request request, Realm realm)
                 case DIGEST:
                     if (isNonEmpty(realm.getNonce())) {
                         // update realm with request information
+                        final Uri uri = request.getUri();
                         realm = realm(realm)
-                                .setUri(request.getUri())
+                                .setUri(uri)
                                 .setMethodName(request.getMethod())
                                 .build();
-                        authorizationHeader = computeDigestAuthentication(realm);
+                        authorizationHeader = computeDigestAuthentication(realm, uri);
                     }
                     break;
                 case NTLM:
diff --git a/client/src/main/java/org/asynchttpclient/util/HttpUtils.java b/client/src/main/java/org/asynchttpclient/util/HttpUtils.java
@@ -19,6 +19,7 @@
 import org.asynchttpclient.Param;
 import org.asynchttpclient.Request;
 import org.asynchttpclient.uri.Uri;
+import org.jetbrains.annotations.Nullable;
 
 import java.net.URLEncoder;
 import java.nio.ByteBuffer;
@@ -59,16 +60,16 @@ public static String originHeader(Uri uri) {
         return sb.toString();
     }
 
-    public static Charset extractContentTypeCharsetAttribute(String contentType) {
+    public static @Nullable Charset extractContentTypeCharsetAttribute(String contentType) {
         String charsetName = extractContentTypeAttribute(contentType, CONTENT_TYPE_CHARSET_ATTRIBUTE);
         return charsetName != null ? Charset.forName(charsetName) : null;
     }
 
-    public static String extractContentTypeBoundaryAttribute(String contentType) {
+    public static @Nullable String extractContentTypeBoundaryAttribute(String contentType) {
         return extractContentTypeAttribute(contentType, CONTENT_TYPE_BOUNDARY_ATTRIBUTE);
     }
 
-    private static String extractContentTypeAttribute(String contentType, String attribute) {
+    private static @Nullable String extractContentTypeAttribute(@Nullable String contentType, String attribute) {
         if (contentType == null) {
             return null;
         }
@@ -147,7 +148,7 @@ private static StringBuilder urlEncodeFormParams0(List<Param> params, Charset ch
         return sb;
     }
 
-    private static void encodeAndAppendFormParam(StringBuilder sb, String name, String value, Charset charset) {
+    private static void encodeAndAppendFormParam(StringBuilder sb, String name, @Nullable String value, Charset charset) {
         encodeAndAppendFormField(sb, name, charset);
         if (value != null) {
             sb.append('=');
diff --git a/client/src/main/java/org/asynchttpclient/util/MiscUtils.java b/client/src/main/java/org/asynchttpclient/util/MiscUtils.java
@@ -37,18 +37,22 @@ public static boolean isEmpty(@Nullable String string) {
         return string == null || string.isEmpty();
     }
 
+    @Contract(value = "null -> false", pure = true)
     public static boolean isNonEmpty(@Nullable Object[] array) {
         return array != null && array.length != 0;
     }
 
+    @Contract(value = "null -> false", pure = true)
     public static boolean isNonEmpty(byte @Nullable [] array) {
         return array != null && array.length != 0;
     }
 
+    @Contract(value = "null -> false")
     public static boolean isNonEmpty(@Nullable Collection<?> collection) {
         return collection != null && !collection.isEmpty();
     }
 
+    @Contract(value = "null -> false")
     public static boolean isNonEmpty(@Nullable Map<?, ?> map) {
         return map != null && !map.isEmpty();
     }
diff --git a/client/src/main/java/org/asynchttpclient/util/ProxyUtils.java b/client/src/main/java/org/asynchttpclient/util/ProxyUtils.java
@@ -17,6 +17,7 @@
 import org.asynchttpclient.Request;
 import org.asynchttpclient.proxy.ProxyServer;
 import org.asynchttpclient.proxy.ProxyServerSelector;
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -80,7 +81,7 @@ private ProxyUtils() {
      * @param request the request
      * @return the proxy server to be used for this request (can be null)
      */
-    public static ProxyServer getProxyServer(AsyncHttpClientConfig config, Request request) {
+    public static @Nullable ProxyServer getProxyServer(AsyncHttpClientConfig config, Request request) {
         ProxyServer proxyServer = request.getProxyServer();
         if (proxyServer == null) {
             ProxyServerSelector selector = config.getProxyServerSelector();
diff --git a/client/src/main/java/org/asynchttpclient/util/UriEncoder.java b/client/src/main/java/org/asynchttpclient/util/UriEncoder.java
@@ -17,6 +17,7 @@
 
 import org.asynchttpclient.Param;
 import org.asynchttpclient.uri.Uri;
+import org.jetbrains.annotations.Nullable;
 
 import java.util.List;
 
@@ -31,7 +32,7 @@ public String encodePath(String path) {
             return Utf8UrlEncoder.encodePath(path);
         }
 
-        private void encodeAndAppendQueryParam(final StringBuilder sb, final CharSequence name, final CharSequence value) {
+        private void encodeAndAppendQueryParam(final StringBuilder sb, final CharSequence name, final @Nullable CharSequence value) {
             Utf8UrlEncoder.encodeAndAppendQueryElement(sb, name);
             if (value != null) {
                 sb.append('=');
@@ -81,7 +82,7 @@ public String encodePath(String path) {
             return path;
         }
 
-        private void appendRawQueryParam(StringBuilder sb, String name, String value) {
+        private void appendRawQueryParam(StringBuilder sb, String name, @Nullable String value) {
             sb.append(name);
             if (value != null) {
                 sb.append('=').append(value);
@@ -131,15 +132,15 @@ public static UriEncoder uriEncoder(boolean disableUrlEncoding) {
 
     protected abstract String withoutQueryWithParams(List<Param> queryParams);
 
-    private String withQuery(final String query, final List<Param> queryParams) {
+    private String withQuery(final String query, final @Nullable List<Param> queryParams) {
         return isNonEmpty(queryParams) ? withQueryWithParams(query, queryParams) : withQueryWithoutParams(query);
     }
 
-    private String withoutQuery(final List<Param> queryParams) {
+    private @Nullable String withoutQuery(final @Nullable List<Param> queryParams) {
         return isNonEmpty(queryParams) ? withoutQueryWithParams(queryParams) : null;
     }
 
-    public Uri encode(Uri uri, List<Param> queryParams) {
+    public Uri encode(Uri uri, @Nullable List<Param> queryParams) {
         String newPath = encodePath(uri.getPath());
         String newQuery = encodeQuery(uri.getQuery(), queryParams);
         return new Uri(uri.getScheme(),
@@ -153,7 +154,7 @@ public Uri encode(Uri uri, List<Param> queryParams) {
 
     protected abstract String encodePath(String path);
 
-    private String encodeQuery(final String query, final List<Param> queryParams) {
+    private @Nullable String encodeQuery(final @Nullable String query, final @Nullable List<Param> queryParams) {
         return isNonEmpty(query) ? withQuery(query, queryParams) : withoutQuery(queryParams);
     }
 }
diff --git a/client/src/main/java/org/asynchttpclient/util/Utf8UrlEncoder.java b/client/src/main/java/org/asynchttpclient/util/Utf8UrlEncoder.java
@@ -15,6 +15,9 @@
  */
 package org.asynchttpclient.util;
 
+import org.jetbrains.annotations.Contract;
+import org.jetbrains.annotations.Nullable;
+
 import java.util.BitSet;
 
 public final class Utf8UrlEncoder {
@@ -144,7 +147,8 @@ public static StringBuilder encodeAndAppendFormElement(StringBuilder sb, CharSeq
         return appendEncoded(sb, input, FORM_URL_ENCODED_SAFE_CHARS, true);
     }
 
-    public static String percentEncodeQueryElement(String input) {
+    @Contract(value = "!null -> !null")
+    public static @Nullable String percentEncodeQueryElement(@Nullable String input) {
         if (input == null) {
             return null;
         }
@@ -165,7 +169,7 @@ private static StringBuilder lazyInitStringBuilder(CharSequence input, int first
         return sb;
     }
 
-    private static StringBuilder lazyAppendEncoded(StringBuilder sb, CharSequence input, BitSet dontNeedEncoding, boolean encodeSpaceAsPlus) {
+    private static @Nullable StringBuilder lazyAppendEncoded(@Nullable StringBuilder sb, CharSequence input, BitSet dontNeedEncoding, boolean encodeSpaceAsPlus) {
         int c;
         for (int i = 0; i < input.length(); i += Character.charCount(c)) {
             c = Character.codePointAt(input, i);
diff --git a/pom.xml b/pom.xml
@@ -233,7 +233,7 @@
                             -Xep:NullOptional:ERROR
                             -XepExcludedPaths:.*/src/test/java/.*
                             -XepOpt:NullAway:AnnotatedPackages=org.asynchttpclient
-                            -XepOpt:NullAway:UnannotatedSubPackages=org.asynchttpclient.netty,org.asynchttpclient.ntlm,org.asynchttpclient.request,org.asynchttpclient.spnego,org.asynchttpclient.util,org.asynchttpclient.webdav,org.asynchttpclient.ws
+                            -XepOpt:NullAway:UnannotatedSubPackages=org.asynchttpclient.netty,org.asynchttpclient.ntlm,org.asynchttpclient.request,org.asynchttpclient.spnego,org.asynchttpclient.webdav,org.asynchttpclient.ws
                             -XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true
                             -Xep:NullAway:ERROR
                         </arg>

Original file line number	Diff line number	Diff line change
`@@ -147,7 +147,7 @@ public Builder setSecuredPort(int securedPort) {`
`147`	`147`	`return this;`
`148`	`148`	`}`
`149`	`149`
`150`		`- public Builder setRealm(Realm realm) {`
	`150`	`+ public Builder setRealm(@Nullable Realm realm) {`
`151`	`151`	`this.realm = realm;`
`152`	`152`	`return this;`
`153`	`153`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,18 +37,22 @@ public static boolean isEmpty(@Nullable String string) {`
`37`	`37`	`return string == null \|\| string.isEmpty();`
`38`	`38`	`}`
`39`	`39`
	`40`	`+ @Contract(value = "null -> false", pure = true)`
`40`	`41`	`public static boolean isNonEmpty(@Nullable Object[] array) {`
`41`	`42`	`return array != null && array.length != 0;`
`42`	`43`	`}`
`43`	`44`
	`45`	`+ @Contract(value = "null -> false", pure = true)`
`44`	`46`	`public static boolean isNonEmpty(byte @Nullable [] array) {`
`45`	`47`	`return array != null && array.length != 0;`
`46`	`48`	`}`
`47`	`49`
	`50`	`+ @Contract(value = "null -> false")`
`48`	`51`	`public static boolean isNonEmpty(@Nullable Collection<?> collection) {`
`49`	`52`	`return collection != null && !collection.isEmpty();`
`50`	`53`	`}`
`51`	`54`
	`55`	`+ @Contract(value = "null -> false")`
`52`	`56`	`public static boolean isNonEmpty(@Nullable Map<?, ?> map) {`
`53`	`57`	`return map != null && !map.isEmpty();`
`54`	`58`	`}`