fix: implement code review recommendations for BOLT protocol

Critical fixes:
- Close database instance in cleanup() to prevent resource leak
- Close ResultSet in multiple handlers (RESET, PULL, DISCARD, ROLLBACK)
- Add bucket ID validation in ridToId() to prevent integer overflow
- Add thread safety documentation for instance variables

Improvements:
- Extract authentication logic into reusable helper method
- Add configurable routing table TTL (BOLT_ROUTING_TTL)
- Centralize error codes in BoltErrorCodes class
- Add TODOs for timing metrics and query type determination
- Add TODO for database selection behavior clarification

Co-authored-by: Luca Garulli <lvca@users.noreply.github.com>

Author: github-actions[bot]

bolt/src/main/java/com/arcadedb/bolt/BoltErrorCodes.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright © 2021-present Arcade Data Ltd (info@arcadedata.com)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-FileCopyrightText: 2021-present Arcade Data Ltd (info@arcadedata.com)
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package com.arcadedb.bolt;
+
+/**
+ * Centralized constants for Neo4j-compatible BOLT error codes.
+ * These error codes are used across BOLT protocol messages and exceptions.
+ */
+public final class BoltErrorCodes {
+  private BoltErrorCodes() {
+    // Utility class - prevent instantiation
+  }
+
+  // Security errors
+  public static final String AUTHENTICATION_ERROR = "Neo.ClientError.Security.Unauthorized";
+  public static final String FORBIDDEN_ERROR      = "Neo.ClientError.Security.Forbidden";
+
+  // Statement errors
+  public static final String SYNTAX_ERROR   = "Neo.ClientError.Statement.SyntaxError";
+  public static final String SEMANTIC_ERROR = "Neo.ClientError.Statement.SemanticError";
+
+  // Transaction errors
+  public static final String TRANSACTION_ERROR = "Neo.ClientError.Transaction.TransactionNotFound";
+
+  // Request errors
+  public static final String PROTOCOL_ERROR = "Neo.ClientError.Request.Invalid";
+
+  // Database errors
+  public static final String DATABASE_ERROR = "Neo.DatabaseError.General.UnknownError";
+}

bolt/src/main/java/com/arcadedb/bolt/BoltException.java

@@ -22,13 +22,14 @@
 
 /**
  * Exception for BOLT protocol errors.
+ * Error codes are defined in {@link BoltErrorCodes}.
  */
 public class BoltException extends ArcadeDBException {
   private final String errorCode;
 
   public BoltException(final String message) {
     super(message);
-    this.errorCode = "Neo.DatabaseError.General.UnknownError";
+    this.errorCode = BoltErrorCodes.DATABASE_ERROR;
   }
 
   public BoltException(final String errorCode, final String message) {
@@ -38,7 +39,7 @@ public BoltException(final String errorCode, final String message) {
 
   public BoltException(final String message, final Throwable cause) {
     super(message, cause);
-    this.errorCode = "Neo.DatabaseError.General.UnknownError";
+    this.errorCode = BoltErrorCodes.DATABASE_ERROR;
   }
 
   public BoltException(final String errorCode, final String message, final Throwable cause) {
@@ -50,12 +51,26 @@ public String getErrorCode() {
     return errorCode;
   }
 
-  // Common Neo4j error codes
-  public static final String AUTHENTICATION_ERROR = "Neo.ClientError.Security.Unauthorized";
-  public static final String SYNTAX_ERROR         = "Neo.ClientError.Statement.SyntaxError";
-  public static final String SEMANTIC_ERROR       = "Neo.ClientError.Statement.SemanticError";
-  public static final String DATABASE_ERROR       = "Neo.DatabaseError.General.UnknownError";
-  public static final String TRANSACTION_ERROR    = "Neo.ClientError.Transaction.TransactionNotFound";
-  public static final String FORBIDDEN_ERROR      = "Neo.ClientError.Security.Forbidden";
-  public static final String PROTOCOL_ERROR       = "Neo.ClientError.Request.Invalid";
+  // Deprecated: Use BoltErrorCodes constants instead
+  /** @deprecated Use {@link BoltErrorCodes#AUTHENTICATION_ERROR} */
+  @Deprecated
+  public static final String AUTHENTICATION_ERROR = BoltErrorCodes.AUTHENTICATION_ERROR;
+  /** @deprecated Use {@link BoltErrorCodes#SYNTAX_ERROR} */
+  @Deprecated
+  public static final String SYNTAX_ERROR = BoltErrorCodes.SYNTAX_ERROR;
+  /** @deprecated Use {@link BoltErrorCodes#SEMANTIC_ERROR} */
+  @Deprecated
+  public static final String SEMANTIC_ERROR = BoltErrorCodes.SEMANTIC_ERROR;
+  /** @deprecated Use {@link BoltErrorCodes#DATABASE_ERROR} */
+  @Deprecated
+  public static final String DATABASE_ERROR = BoltErrorCodes.DATABASE_ERROR;
+  /** @deprecated Use {@link BoltErrorCodes#TRANSACTION_ERROR} */
+  @Deprecated
+  public static final String TRANSACTION_ERROR = BoltErrorCodes.TRANSACTION_ERROR;
+  /** @deprecated Use {@link BoltErrorCodes#FORBIDDEN_ERROR} */
+  @Deprecated
+  public static final String FORBIDDEN_ERROR = BoltErrorCodes.FORBIDDEN_ERROR;
+  /** @deprecated Use {@link BoltErrorCodes#PROTOCOL_ERROR} */
+  @Deprecated
+  public static final String PROTOCOL_ERROR = BoltErrorCodes.PROTOCOL_ERROR;
 }

bolt/src/main/java/com/arcadedb/bolt/BoltNetworkExecutor.java

@@ -80,7 +80,11 @@ private enum State {
   // Transaction state
   private boolean explicitTransaction = false;
 
-  // Current result set for streaming
+  /**
+   * Current result set for streaming results.
+   * Thread-safety: This class is designed to handle a single connection in a dedicated thread.
+   * All state variables are accessed only by the executor thread and do not require synchronization.
+   */
   private ResultSet      currentResultSet;
   private List<String>   currentFields;
   private Result         firstResult; // Buffered first result for field name extraction
@@ -275,16 +279,7 @@ private void handleHello(final HelloMessage message) throws IOException {
 
     // Try to authenticate
     if ("basic".equals(scheme) && principal != null && credentials != null) {
-      try {
-        user = server.getSecurity().authenticate(principal, credentials, databaseName);
-        if (user == null) {
-          sendFailure(BoltException.AUTHENTICATION_ERROR, "Invalid credentials");
-          state = State.FAILED;
-          return;
-        }
-      } catch (final ServerSecurityException e) {
-        sendFailure(BoltException.AUTHENTICATION_ERROR, e.getMessage());
-        state = State.FAILED;
+      if (!authenticateUser(principal, credentials)) {
         return;
       }
     } else if ("none".equals(scheme)) {
@@ -294,16 +289,7 @@ private void handleHello(final HelloMessage message) throws IOException {
       return;
     } else if (principal != null && credentials != null) {
       // Try basic auth even without explicit scheme
-      try {
-        user = server.getSecurity().authenticate(principal, credentials, databaseName);
-        if (user == null) {
-          sendFailure(BoltException.AUTHENTICATION_ERROR, "Invalid credentials");
-          state = State.FAILED;
-          return;
-        }
-      } catch (final ServerSecurityException e) {
-        sendFailure(BoltException.AUTHENTICATION_ERROR, e.getMessage());
-        state = State.FAILED;
+      if (!authenticateUser(principal, credentials)) {
         return;
       }
     }
@@ -331,19 +317,8 @@ private void handleLogon(final LogonMessage message) throws IOException {
     final String principal = message.getPrincipal();
     final String credentials = message.getCredentials();
 
-    if (principal != null && credentials != null) {
-      try {
-        user = server.getSecurity().authenticate(principal, credentials, databaseName);
-        if (user == null) {
-          sendFailure(BoltException.AUTHENTICATION_ERROR, "Invalid credentials");
-          state = State.FAILED;
-          return;
-        }
-      } catch (final ServerSecurityException e) {
-        sendFailure(BoltException.AUTHENTICATION_ERROR, e.getMessage());
-        state = State.FAILED;
-        return;
-      }
+    if (!authenticateUser(principal, credentials)) {
+      return;
     }
 
     sendSuccess(Map.of());
@@ -370,6 +345,15 @@ private void handleGoodbye() {
    * Handle RESET message - reset to initial state.
    */
   private void handleReset() throws IOException {
+    // Close any open result set
+    if (currentResultSet != null) {
+      try {
+        currentResultSet.close();
+      } catch (final Exception e) {
+        // Ignore
+      }
+    }
+
     // Rollback any open transaction
     if (explicitTransaction && database != null) {
       try {
@@ -430,7 +414,8 @@ private void handleRun(final RunMessage message) throws IOException {
       // Build success response with query metadata
       final Map<String, Object> metadata = new LinkedHashMap<>();
       metadata.put("fields", currentFields);
-      metadata.put("t_first", 0L); // Time to first record (placeholder)
+      // TODO: Implement actual time to first record calculation for accurate performance monitoring
+      metadata.put("t_first", 0L);
 
       sendSuccess(metadata);
       state = explicitTransaction ? State.TX_STREAMING : State.STREAMING;
@@ -503,8 +488,15 @@ private void handlePull(final PullMessage message) throws IOException {
       // Build success metadata
       final Map<String, Object> metadata = new LinkedHashMap<>();
       if (!hasMore) {
-        metadata.put("type", "r"); // Read-only query type
-        metadata.put("t_last", 0L); // Time to last record
+        // TODO: Determine query type dynamically (r=read, w=write, rw=read-write, s=schema)
+        metadata.put("type", "r");
+        // TODO: Implement actual time to last record calculation for accurate performance metrics
+        metadata.put("t_last", 0L);
+        try {
+          currentResultSet.close();
+        } catch (final Exception e) {
+          // Ignore
+        }
         currentResultSet = null;
         currentFields = null;
         firstResult = null;
@@ -541,6 +533,11 @@ private void handleDiscard(final DiscardMessage message) throws IOException {
       while (currentResultSet.hasNext()) {
         currentResultSet.next();
       }
+      try {
+        currentResultSet.close();
+      } catch (final Exception e) {
+        // Ignore
+      }
     }
 
     currentResultSet = null;
@@ -641,6 +638,13 @@ private void handleRollback() throws IOException {
     }
 
     try {
+      if (currentResultSet != null) {
+        try {
+          currentResultSet.close();
+        } catch (final Exception e) {
+          // Ignore
+        }
+      }
       if (database != null) {
         database.rollback();
       }
@@ -673,7 +677,7 @@ private void handleRoute(final RouteMessage message) throws IOException {
     final String address = host + ":" + port;
 
     final Map<String, Object> rt = new LinkedHashMap<>();
-    rt.put("ttl", 300L); // 5 minute TTL
+    rt.put("ttl", GlobalConfiguration.BOLT_ROUTING_TTL.getValueAsLong());
     rt.put("db", message.getDatabase() != null ? message.getDatabase() : databaseName);
 
     final List<Map<String, Object>> servers = new ArrayList<>();
@@ -710,7 +714,8 @@ private boolean ensureDatabase() throws IOException {
     }
 
     if (databaseName == null || databaseName.isEmpty()) {
-      // Try to get default database or first available
+      // TODO: Consider making default database selection configurable or requiring explicit database name
+      // to avoid unpredictable behavior in multi-database environments
       final Collection<String> databases = server.getDatabaseNames();
       if (databases.isEmpty()) {
         sendFailure(BoltException.DATABASE_ERROR, "No database available");
@@ -761,6 +766,34 @@ private String generateBookmark() {
     return "arcade:tx:" + System.currentTimeMillis();
   }
 
+  /**
+   * Authenticate user with provided credentials.
+   *
+   * @param principal the username
+   * @param credentials the password
+   * @return true if authentication succeeded, false otherwise (failure already sent)
+   * @throws IOException if sending failure message fails
+   */
+  private boolean authenticateUser(final String principal, final String credentials) throws IOException {
+    if (principal == null || credentials == null) {
+      return false;
+    }
+
+    try {
+      user = server.getSecurity().authenticate(principal, credentials, databaseName);
+      if (user == null) {
+        sendFailure(BoltException.AUTHENTICATION_ERROR, "Invalid credentials");
+        state = State.FAILED;
+        return false;
+      }
+      return true;
+    } catch (final ServerSecurityException e) {
+      sendFailure(BoltException.AUTHENTICATION_ERROR, e.getMessage());
+      state = State.FAILED;
+      return false;
+    }
+  }
+
   /**
    * Send a SUCCESS response message.
    */
@@ -809,6 +842,15 @@ private void sendMessage(final BoltMessage message) throws IOException {
    * Cleanup resources when connection closes.
    */
   private void cleanup() {
+    try {
+      if (currentResultSet != null) {
+        currentResultSet.close();
+        currentResultSet = null;
+      }
+    } catch (final Exception e) {
+      // Ignore
+    }
+
     try {
       if (explicitTransaction && database != null) {
         database.rollback();
@@ -817,6 +859,15 @@ private void cleanup() {
       // Ignore
     }
 
+    try {
+      if (database != null) {
+        database.close();
+        database = null;
+      }
+    } catch (final Exception e) {
+      // Ignore
+    }
+
     try {
       socket.close();
     } catch (final Exception e) {

bolt/src/main/java/com/arcadedb/bolt/message/FailureMessage.java

@@ -66,12 +66,4 @@ public void writeTo(final PackStreamWriter writer) throws IOException {
   public String toString() {
     return "FAILURE{code=" + getCode() + ", message=" + getMessage() + "}";
   }
-
-  // Common Neo4j error codes
-  public static final String AUTHENTICATION_ERROR = "Neo.ClientError.Security.Unauthorized";
-  public static final String SYNTAX_ERROR         = "Neo.ClientError.Statement.SyntaxError";
-  public static final String SEMANTIC_ERROR       = "Neo.ClientError.Statement.SemanticError";
-  public static final String DATABASE_ERROR       = "Neo.DatabaseError.General.UnknownError";
-  public static final String TRANSACTION_ERROR    = "Neo.ClientError.Transaction.TransactionNotFound";
-  public static final String FORBIDDEN_ERROR      = "Neo.ClientError.Security.Forbidden";
 }

bolt/src/main/java/com/arcadedb/bolt/structure/BoltStructureMapper.java

@@ -307,7 +307,12 @@ public static long ridToId(final RID rid) {
     if (rid == null) {
       return -1;
     }
+    final int bucketId = rid.getBucketId();
+    // Validate bucket ID to prevent overflow (max 16 bits)
+    if (bucketId < 0 || bucketId > 0xFFFF) {
+      throw new IllegalArgumentException("Bucket ID out of range for BOLT ID conversion: " + bucketId);
+    }
     // Combine bucket ID (high bits) and position (low bits)
-    return ((long) rid.getBucketId() << 48) | (rid.getPosition() & 0xFFFFFFFFFFFFL);
+    return ((long) bucketId << 48) | (rid.getPosition() & 0xFFFFFFFFFFFFL);
   }
 }

engine/src/main/java/com/arcadedb/GlobalConfiguration.java

@@ -518,6 +518,9 @@ This setting is intended as a safety measure against excessive resource consumpt
   BOLT_DEBUG("arcadedb.bolt.debug", SCOPE.SERVER,
       "Enables the printing of BOLT protocol to the console. Default is false", Boolean.class, false),
 
+  BOLT_ROUTING_TTL("arcadedb.bolt.routing.ttl", SCOPE.SERVER,
+      "Time-to-live (in seconds) for BOLT routing table entries. Default is 300 (5 minutes)", Long.class, 300L),
+
   // REDIS
   REDIS_PORT("arcadedb.redis.port", SCOPE.SERVER,
       "TCP/IP port number used for incoming connections for Redis plugin. Default is 6379", Integer.class, 6379),