Fail correctly when connecting to a server with an unknown server identifier #542
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
technige
changed the title
lutovich
reviewed
Oct 25, 2018
| @@ -71,11 +72,25 @@ public void onRecord( Value[] fields ) | |||
| throw new UnsupportedOperationException(); | |||
| } | |||
|
|
|||
| private static ServerVersion extractServerVersion( Map<String,Value> metadata ) | |||
| private static ServerVersion extractServerVersion( Map<String,Value> metadata ) throws UntrustedServerException | |||
There was a problem hiding this comment.
throws can be removed because UntrustedServerException is a runtime exception
| } | ||
| else | ||
| { | ||
| throw new UntrustedServerException( "Server does not identify as a genuine Neo4j instance" ); |
There was a problem hiding this comment.
Would be nice to include the received product string in the error message
|
|
||
| private static final String NEO4J_IN_DEV_VERSION_STRING = "Neo4j/dev"; | ||
| private static final Pattern PATTERN = | ||
| Pattern.compile( "(Neo4j/)?(\\d+)\\.(\\d+)(?:\\.)?(\\d*)(\\.|-|\\+)?([0-9A-Za-z-.]*)?" ); | ||
| Pattern.compile( "([^/]*)/(\\d+)\\.(\\d+)(?:\\.)?(\\d*)(\\.|-|\\+)?([0-9A-Za-z-.]*)?" ); |
There was a problem hiding this comment.
This will match strings like "/3.5.0" where the product part is absent. It is okay?
| @@ -141,6 +142,10 @@ public boolean lessThanOrEqual(ServerVersion other) | |||
|
|
|||
| private int compareTo( ServerVersion o ) | |||
| { | |||
| if ( !product.equals( o.product ) ) | |||
| { | |||
| throw new IllegalArgumentException("Comparing different products"); | |||
There was a problem hiding this comment.
Would be nice to include product names in the error message
| public static final ServerVersion v3_1_0 = new ServerVersion( 3, 1, 0 ); | ||
| public static final ServerVersion v3_0_0 = new ServerVersion( 3, 0, 0 ); | ||
| public static final ServerVersion vInDev = new ServerVersion( Integer.MAX_VALUE, Integer.MAX_VALUE, Integer.MAX_VALUE ); | ||
| public static final ServerVersion v3_5_0 = new ServerVersion( "Neo4j", 3, 5, 0 ); |
There was a problem hiding this comment.
Constant for string "Neo4j" would be nice
Make version parser not accept version strings like "/3.5.0".
lutovich
force-pushed
the
1.7-block-untrusted-servers
branch
from
9ebba3d to
1f1898e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While we currently parse the "server" string returned in the INIT metadata to extract the server version, no logic existed to validate the product part of that string. Indeed, an unexpected product name would have caused the internal regex to fail and an unhelpful error would surface. This PR raises a new
UntrustedServerExceptionin the case that the product string does not identifyNeo4j.