Enabled NullAway for spnego and webdav packages (AsyncHttpClient#1886)

Asapin · web-flow · commit 8f8b63754726 · 2023-06-05T15:24:17.000+05:30
* Enabled NullAway for WebDav package
* Enabled NullAway for spnego package
diff --git a/client/src/main/java/org/asynchttpclient/spnego/NamePasswordCallbackHandler.java b/client/src/main/java/org/asynchttpclient/spnego/NamePasswordCallbackHandler.java
@@ -15,6 +15,7 @@
  */
 package org.asynchttpclient.spnego;
 
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -32,14 +33,14 @@ public class NamePasswordCallbackHandler implements CallbackHandler {
     private static final Class<?>[] PASSWORD_CALLBACK_TYPES = new Class<?>[]{Object.class, char[].class, String.class};
 
     private final String username;
-    private final String password;
-    private final String passwordCallbackName;
+    private final @Nullable String password;
+    private final @Nullable String passwordCallbackName;
 
-    public NamePasswordCallbackHandler(String username, String password) {
+    public NamePasswordCallbackHandler(String username, @Nullable String password) {
         this(username, password, null);
     }
 
-    public NamePasswordCallbackHandler(String username, String password, String passwordCallbackName) {
+    public NamePasswordCallbackHandler(String username, @Nullable String password, @Nullable String passwordCallbackName) {
         this.username = username;
         this.password = password;
         this.passwordCallbackName = passwordCallbackName;
@@ -54,7 +55,7 @@ public void handle(Callback[] callbacks) throws IOException, UnsupportedCallback
                 ((NameCallback) callback).setName(username);
             } else if (callback instanceof PasswordCallback) {
                 PasswordCallback pwCallback = (PasswordCallback) callback;
-                pwCallback.setPassword(password.toCharArray());
+                pwCallback.setPassword(password != null ? password.toCharArray() : null);
             } else if (!invokePasswordCallback(callback)) {
                 String errorMsg = "Unsupported callback type " + callback.getClass().getName();
                 log.info(errorMsg);
@@ -80,7 +81,7 @@ private boolean invokePasswordCallback(Callback callback) {
             try {
                 Method method = callback.getClass().getMethod(cbname, arg);
                 Object[] args = {
-                        arg == String.class ? password : password.toCharArray()
+                        arg == String.class ? password : password != null ? password.toCharArray() : null
                 };
                 method.invoke(callback, args);
                 return true;
diff --git a/client/src/main/java/org/asynchttpclient/spnego/SpnegoEngine.java b/client/src/main/java/org/asynchttpclient/spnego/SpnegoEngine.java
@@ -42,6 +42,7 @@
 import org.ietf.jgss.GSSManager;
 import org.ietf.jgss.GSSName;
 import org.ietf.jgss.Oid;
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -70,17 +71,19 @@ public class SpnegoEngine {
     private static final String KERBEROS_OID = "1.2.840.113554.1.2.2";
     private static final Map<String, SpnegoEngine> instances = new HashMap<>();
     private final Logger log = LoggerFactory.getLogger(getClass());
-    private final SpnegoTokenGenerator spnegoGenerator;
-    private final String username;
-    private final String password;
-    private final String servicePrincipalName;
-    private final String realmName;
+    private final @Nullable SpnegoTokenGenerator spnegoGenerator;
+    private final @Nullable String username;
+    private final @Nullable String password;
+    private final @Nullable String servicePrincipalName;
+    private final @Nullable String realmName;
     private final boolean useCanonicalHostname;
-    private final String loginContextName;
-    private final Map<String, String> customLoginConfig;
+    private final @Nullable String loginContextName;
+    private final @Nullable Map<String, String> customLoginConfig;
 
-    public SpnegoEngine(final String username, final String password, final String servicePrincipalName, final String realmName, final boolean useCanonicalHostname,
-                        final Map<String, String> customLoginConfig, final String loginContextName, final SpnegoTokenGenerator spnegoGenerator) {
+    public SpnegoEngine(final @Nullable String username, final @Nullable String password,
+                        final @Nullable String servicePrincipalName, final @Nullable String realmName,
+                        final boolean useCanonicalHostname, final @Nullable Map<String, String> customLoginConfig,
+                        final @Nullable String loginContextName, final @Nullable SpnegoTokenGenerator spnegoGenerator) {
         this.username = username;
         this.password = password;
         this.servicePrincipalName = servicePrincipalName;
@@ -95,8 +98,10 @@ public SpnegoEngine() {
         this(null, null, null, null, true, null, null, null);
     }
 
-    public static SpnegoEngine instance(final String username, final String password, final String servicePrincipalName, final String realmName,
-                                        final boolean useCanonicalHostname, final Map<String, String> customLoginConfig, final String loginContextName) {
+    public static SpnegoEngine instance(final @Nullable String username, final @Nullable String password,
+                                        final @Nullable String servicePrincipalName, final @Nullable String realmName,
+                                        final boolean useCanonicalHostname, final @Nullable Map<String, String> customLoginConfig,
+                                        final @Nullable String loginContextName) {
         String key = "";
         if (customLoginConfig != null && !customLoginConfig.isEmpty()) {
             StringBuilder customLoginConfigKeyValues = new StringBuilder();
@@ -151,7 +156,6 @@ public String generateToken(String host) throws SpnegoEngineException {
             // Try SPNEGO by default, fall back to Kerberos later if error
             negotiationOid = new Oid(SPNEGO_OID);
 
-            boolean tryKerberos = false;
             String spn = getCompleteServicePrincipalName(host);
             try {
                 GSSManager manager = GSSManager.getInstance();
@@ -181,13 +185,12 @@ public String generateToken(String host) throws SpnegoEngineException {
                 // Rethrow any other exception.
                 if (ex.getMajor() == GSSException.BAD_MECH) {
                     log.debug("GSSException BAD_MECH, retry with Kerberos MECH");
-                    tryKerberos = true;
                 } else {
                     throw ex;
                 }
 
             }
-            if (tryKerberos) {
+            if (gssContext == null) {
                 /* Kerberos v5 GSS-API mechanism defined in RFC 1964. */
                 log.debug("Using Kerberos MECH {}", KERBEROS_OID);
                 negotiationOid = new Oid(KERBEROS_OID);
@@ -270,14 +273,14 @@ private String getCanonicalHostname(String hostname) {
         return canonicalHostname;
     }
 
-    private CallbackHandler getUsernamePasswordHandler() {
+    private @Nullable CallbackHandler getUsernamePasswordHandler() {
         if (username == null) {
             return null;
         }
         return new NamePasswordCallbackHandler(username, password);
     }
 
-    public Configuration getLoginConfiguration() {
+    public @Nullable Configuration getLoginConfiguration() {
         if (customLoginConfig != null && !customLoginConfig.isEmpty()) {
             return new Configuration() {
                 @Override
diff --git a/client/src/main/java/org/asynchttpclient/spnego/SpnegoEngineException.java b/client/src/main/java/org/asynchttpclient/spnego/SpnegoEngineException.java
@@ -15,18 +15,20 @@
  */
 package org.asynchttpclient.spnego;
 
+import org.jetbrains.annotations.Nullable;
+
 /**
  * Signals SPNEGO protocol failure.
  */
 public class SpnegoEngineException extends Exception {
 
     private static final long serialVersionUID = -3123799505052881438L;
 
-    public SpnegoEngineException(String message) {
+    public SpnegoEngineException(@Nullable String message) {
         super(message);
     }
 
-    public SpnegoEngineException(String message, Throwable cause) {
+    public SpnegoEngineException(@Nullable String message, Throwable cause) {
         super(message, cause);
     }
 }
diff --git a/client/src/main/java/org/asynchttpclient/webdav/WebDavCompletionHandlerBase.java b/client/src/main/java/org/asynchttpclient/webdav/WebDavCompletionHandlerBase.java
@@ -18,6 +18,7 @@
 import org.asynchttpclient.HttpResponseStatus;
 import org.asynchttpclient.Response;
 import org.asynchttpclient.netty.NettyResponse;
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
@@ -45,8 +46,8 @@ public abstract class WebDavCompletionHandlerBase<T> implements AsyncHandler<T>
     private static final Logger LOGGER = LoggerFactory.getLogger(WebDavCompletionHandlerBase.class);
     private static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY;
     private final List<HttpResponseBodyPart> bodyParts = Collections.synchronizedList(new ArrayList<>());
-    private HttpResponseStatus status;
-    private HttpHeaders headers;
+    private @Nullable HttpResponseStatus status;
+    private @Nullable HttpHeaders headers;
 
     static {
         DOCUMENT_BUILDER_FACTORY = DocumentBuilderFactory.newInstance();
@@ -78,19 +79,20 @@ public final State onHeadersReceived(final HttpHeaders headers) {
         return State.CONTINUE;
     }
 
-    private Document readXMLResponse(InputStream stream) {
+    private Document readXMLResponse(InputStream stream, HttpResponseStatus initialStatus) {
         Document document;
         try {
             document = DOCUMENT_BUILDER_FACTORY.newDocumentBuilder().parse(stream);
-            parse(document);
+            status = parse(document, initialStatus);
         } catch (SAXException | IOException | ParserConfigurationException e) {
             LOGGER.error(e.getMessage(), e);
             throw new RuntimeException(e);
         }
         return document;
     }
 
-    private void parse(Document document) {
+    private static HttpResponseStatus parse(Document document, HttpResponseStatus initialStatus) {
+        HttpResponseStatus status = initialStatus;
         Element element = document.getDocumentElement();
         NodeList statusNode = element.getElementsByTagName("status");
         for (int i = 0; i < statusNode.getLength(); i++) {
@@ -101,14 +103,15 @@ private void parse(Document document) {
             String statusText = value.substring(value.lastIndexOf(' '));
             status = new HttpStatusWrapper(status, statusText, statusCode);
         }
+        return status;
     }
 
     @Override
     public final T onCompleted() throws Exception {
         if (status != null) {
             Document document = null;
             if (status.getStatusCode() == 207) {
-                document = readXMLResponse(new NettyResponse(status, headers, bodyParts).getResponseBodyAsStream());
+                document = readXMLResponse(new NettyResponse(status, headers, bodyParts).getResponseBodyAsStream(), status);
             }
             // recompute response as readXMLResponse->parse might have updated it
             return onCompleted(new WebDavResponse(new NettyResponse(status, headers, bodyParts), document));
diff --git a/client/src/main/java/org/asynchttpclient/webdav/WebDavResponse.java b/client/src/main/java/org/asynchttpclient/webdav/WebDavResponse.java
@@ -16,6 +16,7 @@
 import io.netty.handler.codec.http.cookie.Cookie;
 import org.asynchttpclient.Response;
 import org.asynchttpclient.uri.Uri;
+import org.jetbrains.annotations.Nullable;
 import org.w3c.dom.Document;
 
 import java.io.InputStream;
@@ -30,9 +31,9 @@
 public class WebDavResponse implements Response {
 
     private final Response response;
-    private final Document document;
+    private final @Nullable Document document;
 
-    WebDavResponse(Response response, Document document) {
+    WebDavResponse(Response response, @Nullable Document document) {
         this.response = response;
         this.document = document;
     }
@@ -132,7 +133,7 @@ public SocketAddress getLocalAddress() {
         return response.getLocalAddress();
     }
 
-    public Document getBodyAsXML() {
+    public @Nullable Document getBodyAsXML() {
         return document;
     }
 }
diff --git a/client/src/test/java/org/asynchttpclient/spnego/SpnegoEngineTest.java b/client/src/test/java/org/asynchttpclient/spnego/SpnegoEngineTest.java
@@ -21,6 +21,7 @@
 import org.asynchttpclient.AbstractBasicTest;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 import java.io.File;
 import java.util.HashMap;
@@ -96,6 +97,19 @@ public void testSpnegoGenerateTokenWithUsernamePassword() throws Exception {
         assertTrue(token.startsWith("YII"));
     }
 
+    @Test
+    public void testSpnegoGenerateTokenWithNullPasswordFail() {
+        SpnegoEngine spnegoEngine = new SpnegoEngine("alice",
+                null,
+                "bob",
+                "service.ws.apache.org",
+                false,
+                null,
+                "alice",
+                null);
+        assertThrows(SpnegoEngineException.class, () -> spnegoEngine.generateToken("localhost"), "No password provided");
+    }
+
     @RepeatedIfExceptionsTest(repeats = 5)
     public void testSpnegoGenerateTokenWithUsernamePasswordFail() throws Exception {
         SpnegoEngine spnegoEngine = new SpnegoEngine("alice",
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.webdav,org.asynchttpclient.ws
+                            -XepOpt:NullAway:UnannotatedSubPackages=org.asynchttpclient.netty,org.asynchttpclient.ntlm,org.asynchttpclient.request,org.asynchttpclient.ws
                             -XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true
                             -Xep:NullAway:ERROR
                         </arg>

Original file line number	Diff line number	Diff line change
`@@ -15,18 +15,20 @@`
`15`	`15`	`*/`
`16`	`16`	`package org.asynchttpclient.spnego;`
`17`	`17`
	`18`	`+import org.jetbrains.annotations.Nullable;`
	`19`	`+`
`18`	`20`	`/**`
`19`	`21`	`* Signals SPNEGO protocol failure.`
`20`	`22`	`*/`
`21`	`23`	`public class SpnegoEngineException extends Exception {`
`22`	`24`
`23`	`25`	`private static final long serialVersionUID = -3123799505052881438L;`
`24`	`26`
`25`		`- public SpnegoEngineException(String message) {`
	`27`	`+ public SpnegoEngineException(@Nullable String message) {`
`26`	`28`	`super(message);`
`27`	`29`	`}`
`28`	`30`
`29`		`- public SpnegoEngineException(String message, Throwable cause) {`
	`31`	`+ public SpnegoEngineException(@Nullable String message, Throwable cause) {`
`30`	`32`	`super(message, cause);`
`31`	`33`	`}`
`32`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@`
`16`	`16`	`import io.netty.handler.codec.http.cookie.Cookie;`
`17`	`17`	`import org.asynchttpclient.Response;`
`18`	`18`	`import org.asynchttpclient.uri.Uri;`
	`19`	`+import org.jetbrains.annotations.Nullable;`
`19`	`20`	`import org.w3c.dom.Document;`
`20`	`21`
`21`	`22`	`import java.io.InputStream;`
`@@ -30,9 +31,9 @@`
`30`	`31`	`public class WebDavResponse implements Response {`
`31`	`32`
`32`	`33`	`private final Response response;`
`33`		`- private final Document document;`
	`34`	`+ private final @Nullable Document document;`
`34`	`35`
`35`		`- WebDavResponse(Response response, Document document) {`
	`36`	`+ WebDavResponse(Response response, @Nullable Document document) {`
`36`	`37`	`this.response = response;`
`37`	`38`	`this.document = document;`
`38`	`39`	`}`
`@@ -132,7 +133,7 @@ public SocketAddress getLocalAddress() {`
`132`	`133`	`return response.getLocalAddress();`
`133`	`134`	`}`
`134`	`135`
`135`		`- public Document getBodyAsXML() {`
	`136`	`+ public @Nullable Document getBodyAsXML() {`
`136`	`137`	`return document;`
`137`	`138`	`}`
`138`	`139`	`}`