#778 Potential NPE when warningMessageCallback is null

src/docs/asciidoc/release_notes.adoc

@@ -32,6 +32,10 @@ For known issues, consult <<known-issues>>.
 
 The following has been changed or fixed since Jaybird 5.0.3:
 
+* Fixed: Potential NPE when `warningMessageCallback` is `null` while reading operations or consuming packets (https://github.com/FirebirdSQL/jaybird/issues/778[#778])
++
+As part of this change, the constructor `AbstractWireOperations` now explicitly requires the `connection` and `defaultWarningMessageCallback` to be non-``null`` and throws a `NullPointerException` otherwise.
+This may affect custom protocol implementations extending `AbstractWireOperations` and/or calling `ProtocolDescriptor#createWireOperations(WireConnection, WarningMessageCallback)`: make sure you don't pass `null`.
 * ...
 
 [#jaybird-5-0-3-changelog]

src/main/org/firebirdsql/gds/ng/wire/AbstractWireOperations.java

@@ -39,6 +39,7 @@
 import java.sql.SQLWarning;
 import java.util.List;
 
+import static java.util.Objects.requireNonNull;
 import static org.firebirdsql.gds.ISCConstants.*;
 import static org.firebirdsql.gds.impl.wire.WireProtocolConstants.*;
 
@@ -55,8 +56,9 @@ public abstract class AbstractWireOperations implements FbWireOperations {
 
     protected AbstractWireOperations(WireConnection<?, ?> connection,
             WarningMessageCallback defaultWarningMessageCallback) {
-        this.connection = connection;
-        this.defaultWarningMessageCallback = defaultWarningMessageCallback;
+        this.connection = requireNonNull(connection, "connection");
+        this.defaultWarningMessageCallback =
+                requireNonNull(defaultWarningMessageCallback, "defaultWarningMessageCallback");
     }
 
     @Override
@@ -262,18 +264,19 @@ public final void processResponse(Response response) throws SQLException {
      *         Response to process
      */
     public final void processResponseWarnings(final Response response, WarningMessageCallback warningCallback) {
-        if (warningCallback == null) {
-            warningCallback = defaultWarningMessageCallback;
-        }
         if (response instanceof GenericResponse) {
             GenericResponse genericResponse = (GenericResponse) response;
             SQLException exception = genericResponse.getException();
             if (exception instanceof SQLWarning) {
-                warningCallback.processWarning((SQLWarning) exception);
+                orDefault(warningCallback).processWarning((SQLWarning) exception);
             }
         }
     }
 
+    private WarningMessageCallback orDefault(WarningMessageCallback warningMessageCallback) {
+        return warningMessageCallback != null ? warningMessageCallback : defaultWarningMessageCallback;
+    }
+
     @Override
     public final GenericResponse readGenericResponse(WarningMessageCallback warningCallback)
             throws SQLException, IOException {
@@ -297,7 +300,7 @@ public final void consumePackets(int numberOfResponses, WarningMessageCallback w
             try {
                 readResponse(warningCallback);
             } catch (Exception e) {
-                warningCallback.processWarning(new SQLWarning(e));
+                orDefault(warningCallback).processWarning(new SQLWarning(e));
                 // ignoring exceptions
                 log.warnDebug("Exception in consumePackets", e);
             }

src/main/org/firebirdsql/gds/ng/wire/ProtocolDescriptor.java

@@ -214,9 +214,9 @@ FbWireBlob createInputBlob(FbWireDatabase database, FbWireTransaction transactio
      * Create an {@link FbWireOperations} implementation for this protocol version.
      *
      * @param connection
-     *         WireConnection instance
+     *         WireConnection instance (non-{@code null})
      * @param defaultWarningMessageCallback
-     *         Default warning message callback
+     *         default warning message callback (non-{@code null})
      * @return Wire operations implementation
      */
     FbWireOperations createWireOperations(WireConnection<?, ?> connection,

src/main/org/firebirdsql/gds/ng/wire/WireConnection.java

@@ -33,6 +33,7 @@
 import org.firebirdsql.gds.ng.IAttachProperties;
 import org.firebirdsql.gds.ng.IConnectionProperties;
 import org.firebirdsql.gds.ng.LockCloseable;
+import org.firebirdsql.gds.ng.WarningMessageCallback;
 import org.firebirdsql.gds.ng.dbcrypt.DbCryptCallback;
 import org.firebirdsql.gds.ng.wire.auth.ClientAuthBlock;
 import org.firebirdsql.gds.ng.wire.crypt.KnownServerKey;
@@ -75,6 +76,7 @@ public abstract class WireConnection<T extends IAttachProperties<T>, C extends F
     // TODO Check if methods currently throwing IOException should throw SQLException instead
 
     private static final Logger log = LoggerFactory.getLogger(WireConnection.class);
+    private static final WarningMessageCallback NOOP_WARNING_MESSAGE_CALLBACK = warning -> {};
 
     // Micro-optimization: we usually expect at most 3 (Firebird 5)
     private final List<KnownServerKey> knownServerKeys = new ArrayList<>(3);
@@ -487,7 +489,7 @@ void clearServerKeys() {
     private AbstractWireOperations getDefaultWireOperations() {
         ProtocolDescriptor protocolDescriptor = protocols
                 .getProtocolDescriptor(WireProtocolConstants.PROTOCOL_VERSION10);
-        return (AbstractWireOperations) protocolDescriptor.createWireOperations(this, null);
+        return (AbstractWireOperations) protocolDescriptor.createWireOperations(this, NOOP_WARNING_MESSAGE_CALLBACK);
     }
 
     /**
@@ -496,7 +498,7 @@ private AbstractWireOperations getDefaultWireOperations() {
     private FbWireOperations getCryptKeyCallbackWireOperations() {
         ProtocolDescriptor protocolDescriptor = protocols
                 .getProtocolDescriptor(WireProtocolConstants.PROTOCOL_VERSION15);
-        return protocolDescriptor.createWireOperations(this, null);
+        return protocolDescriptor.createWireOperations(this, NOOP_WARNING_MESSAGE_CALLBACK);
     }
 
     /**