Improve handling of malformed BEGIN and RUN metadata

Make metadata parsing errors result in a FAILURE message. Previously, they forced Bolt server to close the connection. Such errors can happen when bookmark format is invalid, transaction timeout or metadata can't be parsed or if of illegal type.
neo4j · Aug 1, 2018 · 60ba42c · 60ba42c
1 parent 8559647
commit 60ba42c
Show file tree

Hide file tree

Showing 9 changed files with 289 additions and 179 deletions.
diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/ReadyState.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/ReadyState.java
@@ -30,7 +30,6 @@
 import org.neo4j.bolt.v1.messaging.request.RunMessage;
 import org.neo4j.bolt.v1.runtime.bookmarking.Bookmark;
 import org.neo4j.graphdb.security.AuthorizationExpiredException;
-import org.neo4j.internal.kernel.api.exceptions.KernelException;
 import org.neo4j.values.storable.Values;
 
 import static org.neo4j.bolt.v1.runtime.RunMessageChecker.isBegin;
@@ -118,7 +117,7 @@ private BoltStateMachineState processRunMessage( RunMessage message, StateMachin
         }
     }
 
-    private static StatementMetadata processRunMessage( RunMessage message, StatementProcessor statementProcessor ) throws KernelException
+    private static StatementMetadata processRunMessage( RunMessage message, StatementProcessor statementProcessor ) throws Exception
     {
         if ( isBegin( message ) )
         {

diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/bookmarking/Bookmark.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/bookmarking/Bookmark.java
@@ -22,8 +22,6 @@
 import java.util.Objects;
 
 import org.neo4j.bolt.runtime.BoltResponseHandler;
-import org.neo4j.internal.kernel.api.exceptions.KernelException;
-import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.values.AnyValue;
 import org.neo4j.values.storable.TextValue;
 import org.neo4j.values.storable.Values;
@@ -37,7 +35,7 @@ public class Bookmark
 {
     private static final String BOOKMARK_KEY = "bookmark";
     private static final String BOOKMARKS_KEY = "bookmarks";
-    private static final String BOOKMARK_TX_PREFIX = "neo4j:bookmark:v1:tx";
+    static final String BOOKMARK_TX_PREFIX = "neo4j:bookmark:v1:tx";
 
     private final long txId;
 
@@ -160,19 +158,4 @@ public void attachTo( BoltResponseHandler state )
     {
         state.onMetadata( BOOKMARK_KEY, stringValue( toString() ) );
     }
-
-    static class BookmarkFormatException extends KernelException
-    {
-        BookmarkFormatException( String bookmarkString, NumberFormatException e )
-        {
-            super( Status.Transaction.InvalidBookmark, e, "Supplied bookmark [%s] does not conform to pattern %s; " +
-                    "unable to parse transaction id", bookmarkString, BOOKMARK_TX_PREFIX );
-        }
-
-        BookmarkFormatException( Object bookmarkObject )
-        {
-            super( Status.Transaction.InvalidBookmark, "Supplied bookmark [%s] does not conform to pattern %s",
-                    bookmarkObject, BOOKMARK_TX_PREFIX );
-        }
-    }
 }
diff --git a/...ity/bolt/src/main/java/org/neo4j/bolt/v1/runtime/bookmarking/BookmarkFormatException.java b/...ity/bolt/src/main/java/org/neo4j/bolt/v1/runtime/bookmarking/BookmarkFormatException.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2002-2018 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.bolt.v1.runtime.bookmarking;
+
+import org.neo4j.bolt.messaging.BoltIOException;
+import org.neo4j.kernel.api.exceptions.Status;
+
+import static org.neo4j.bolt.v1.runtime.bookmarking.Bookmark.BOOKMARK_TX_PREFIX;
+
+class BookmarkFormatException extends BoltIOException
+{
+    BookmarkFormatException( String bookmarkString, NumberFormatException cause )
+    {
+        super( Status.Transaction.InvalidBookmark,
+                String.format( "Supplied bookmark [%s] does not conform to pattern %s; unable to parse transaction id", bookmarkString, BOOKMARK_TX_PREFIX ),
+                cause );
+    }
+
+    BookmarkFormatException( Object bookmarkObject )
+    {
+        super( Status.Transaction.InvalidBookmark,
+                String.format( "Supplied bookmark [%s] does not conform to pattern %s", bookmarkObject, BOOKMARK_TX_PREFIX ) );
+    }
+}
diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/BeginMessage.java b/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/BeginMessage.java
@@ -30,7 +30,6 @@
 import org.neo4j.values.virtual.VirtualValues;
 
 import static java.util.Objects.requireNonNull;
-import static org.neo4j.bolt.v3.messaging.request.MessageMetadataParser.parseBookmark;
 import static org.neo4j.bolt.v3.messaging.request.MessageMetadataParser.parseTransactionMetadata;
 import static org.neo4j.bolt.v3.messaging.request.MessageMetadataParser.parseTransactionTimeout;
 
@@ -51,7 +50,7 @@ public BeginMessage() throws BoltIOException
     public BeginMessage( MapValue meta ) throws BoltIOException
     {
         this.meta = requireNonNull( meta );
-        this.bookmark = parseBookmark( meta );
+        this.bookmark = Bookmark.fromParamsOrNull( meta );
         this.txTimeout = parseTransactionTimeout( meta );
         this.txMetadata = parseTransactionMetadata( meta );
     }
@@ -84,7 +83,7 @@ public boolean equals( Object o )
             return false;
         }
         BeginMessage that = (BeginMessage) o;
-        return Objects.equals( meta, that.meta ) && Objects.equals( meta, that.meta );
+        return Objects.equals( meta, that.meta );
     }
 
     @Override

diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/MessageMetadataParser.java b/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/MessageMetadataParser.java
@@ -24,11 +24,9 @@
 import java.util.Map;
 
 import org.neo4j.bolt.messaging.BoltIOException;
-import org.neo4j.bolt.v1.runtime.bookmarking.Bookmark;
 import org.neo4j.graphdb.Node;
 import org.neo4j.graphdb.Relationship;
 import org.neo4j.graphdb.spatial.Point;
-import org.neo4j.internal.kernel.api.exceptions.KernelException;
 import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.kernel.impl.util.BaseToObjectValueWriter;
 import org.neo4j.values.AnyValue;
@@ -40,21 +38,13 @@
 /**
  * The parsing methods in this class returns null if the specified key is not found in the input message metadata map.
  */
-class MessageMetadataParser
+final class MessageMetadataParser
 {
     private static final String TX_TIMEOUT_KEY = "tx_timeout";
     private static final String TX_META_DATA_KEY = "tx_metadata";
 
-    static Bookmark parseBookmark( MapValue meta ) throws BoltIOException
+    private MessageMetadataParser()
     {
-        try
-        {
-            return Bookmark.fromParamsOrNull( meta );
-        }
-        catch ( KernelException e )
-        {
-            throw new BoltIOException( Status.Request.InvalidFormat, e.getMessage(), e );
-        }
     }
 
     static Duration parseTransactionTimeout( MapValue meta ) throws BoltIOException
@@ -70,7 +60,7 @@ else if ( anyValue instanceof LongValue )
         }
         else
         {
-            throw new BoltIOException( Status.Request.InvalidFormat, "Expecting transaction timeout value to be a Long value, but got: " + anyValue );
+            throw new BoltIOException( Status.Request.Invalid, "Expecting transaction timeout value to be a Long value, but got: " + anyValue );
         }
     }
 
@@ -91,7 +81,7 @@ else if ( anyValue instanceof MapValue )
         }
         else
         {
-            throw new BoltIOException( Status.Request.InvalidFormat, "Expecting transaction metadata value to be a Map value, but got: " + anyValue );
+            throw new BoltIOException( Status.Request.Invalid, "Expecting transaction metadata value to be a Map value, but got: " + anyValue );
         }
     }
 

diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/RunMessage.java b/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/RunMessage.java
@@ -30,7 +30,6 @@
 import org.neo4j.values.virtual.VirtualValues;
 
 import static java.util.Objects.requireNonNull;
-import static org.neo4j.bolt.v3.messaging.request.MessageMetadataParser.parseBookmark;
 import static org.neo4j.bolt.v3.messaging.request.MessageMetadataParser.parseTransactionMetadata;
 import static org.neo4j.bolt.v3.messaging.request.MessageMetadataParser.parseTransactionTimeout;
 
@@ -62,7 +61,7 @@ public RunMessage( String statement, MapValue params, MapValue meta ) throws Bol
         this.params = requireNonNull( params );
         this.meta = requireNonNull( meta );
 
-        this.bookmark = parseBookmark( meta );
+        this.bookmark = Bookmark.fromParamsOrNull( meta );
         this.txTimeout = parseTransactionTimeout( meta );
         this.txMetadata = parseTransactionMetadata( meta );
     }