From 48e4e6db0e694eccc6e025377cac43570724e7d7 Mon Sep 17 00:00:00 2001 From: Aaron Vaage Date: Wed, 19 Apr 2017 17:07:33 -0700 Subject: [PATCH] Fixing Uuid Parse Error In Uuid, ids are 32 bit unsigned integers. As Java does not supported unsigned integers, when converting to a string, the 32 bit integer is converted to a long so that it can be written to a string as a positive integer (using all 32 bits). When converting from a string to a Uuid, the integer value is not handled correctly as values that would be acceptable as a 32 bit unsigned integer fail to be parsed as a 32 bit signed integer. If the id value of a Uuid is 0xFFFFFFFF and is converted to the string 4294967295. When 4294967295 is parsed, it will fail as it is not expressible as a 32 bit signed integer. Parsing needs to parse the unsigned integer and convert and store the bit pattern correctly in a signed integer. Closes #62 --- src/codeu/chat/RelayMain.java | 2 +- src/codeu/chat/ServerMain.java | 10 ++++++++-- src/codeu/chat/util/Uuid.java | 20 +++++++++++++------- test/codeu/chat/util/UuidTest.java | 21 +++++++++++++++++---- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/codeu/chat/RelayMain.java b/src/codeu/chat/RelayMain.java index 28e67be99..2784e6a81 100644 --- a/src/codeu/chat/RelayMain.java +++ b/src/codeu/chat/RelayMain.java @@ -137,7 +137,7 @@ private static void loadTeamInfo(Server relay, String file) { // this line that it is not worth trying to handle ahead of time. // So instead just try to parse it and catch any exception. - final Uuid id = Uuid.fromString(tokens[0].trim()); + final Uuid id = Uuid.parse(tokens[0].trim()); final byte[] secret = Secret.parse(tokens[1].trim()); relay.addTeam(id, secret); diff --git a/src/codeu/chat/ServerMain.java b/src/codeu/chat/ServerMain.java index 5b864b019..2833588ec 100644 --- a/src/codeu/chat/ServerMain.java +++ b/src/codeu/chat/ServerMain.java @@ -46,10 +46,16 @@ public static void main(String[] args) { LOG.info("============================= START OF LOG ============================="); - final Uuid id = Uuid.fromString(args[0]); + final int myPort = Integer.parseInt(args[2]); final byte[] secret = Secret.parse(args[1]); - final int myPort = Integer.parseInt(args[2]); + Uuid id = null; + try { + id = Uuid.parse(args[0]); + } catch (IOException ex) { + System.out.println("Invalid id - shutting down server"); + System.exit(1); + } // This is the directory where it is safe to store data accross runs // of the server. diff --git a/src/codeu/chat/util/Uuid.java b/src/codeu/chat/util/Uuid.java index c02689a34..994dee08f 100644 --- a/src/codeu/chat/util/Uuid.java +++ b/src/codeu/chat/util/Uuid.java @@ -179,23 +179,29 @@ private static void buildString(Uuid current, StringBuilder build) { } } - // FROM STRING + // Parse // // Create a uuid from a sting. - public static Uuid fromString(String string) { - return fromString(null, string.split("\\."), 0); + public static Uuid parse(String string) throws IOException { + return parse(null, string.split("\\."), 0); } - private static Uuid fromString(final Uuid root, String[] tokens, int index) { + private static Uuid parse(final Uuid root, String[] tokens, int index) throws IOException { - final int id = Integer.parseInt(tokens[index]); + final long id = Long.parseLong(tokens[index]); - final Uuid link = new Uuid(root, id); + if ((id >> 32) != 0) { + throw new IOException(String.format( + "ID value '%s' is too large to be an unsigned 32 bit integer", + tokens[index])); + } + + final Uuid link = new Uuid(root, (int)(id & 0xFFFFFFFF)); final int nextIndex = index + 1; return nextIndex < tokens.length ? - fromString(link, tokens, nextIndex) : + parse(link, tokens, nextIndex) : link; } } diff --git a/test/codeu/chat/util/UuidTest.java b/test/codeu/chat/util/UuidTest.java index cf3925e8b..def446928 100644 --- a/test/codeu/chat/util/UuidTest.java +++ b/test/codeu/chat/util/UuidTest.java @@ -14,6 +14,7 @@ package codeu.chat.util; +import java.io.IOException; import static org.junit.Assert.*; import org.junit.Test; @@ -103,10 +104,10 @@ public void testRootEqualNot() { } @Test - public void testValidSingleLink() { + public void testValidSingleLink() throws IOException { final String string = "100"; - final Uuid id = Uuid.fromString(string); + final Uuid id = Uuid.parse(string); assertNotNull(id); assertNull(id.root()); @@ -114,10 +115,10 @@ public void testValidSingleLink() { } @Test - public void testValidMultiLink() { + public void testValidMultiLink() throws IOException { final String string = "100.200"; - final Uuid id = Uuid.fromString(string); + final Uuid id = Uuid.parse(string); assertNotNull(id); assertNotNull(id.root()); @@ -126,4 +127,16 @@ public void testValidMultiLink() { assertEquals(id.id(), 200); assertEquals(id.root().id(), 100); } + + @Test + public void testLargeId() throws IOException { + + // Use a id value that would be too large for Integer.parseInt to handle + // but would still parse if we could use unsigned integers. + final String string = Long.toString(0xFFFFFFFFL); + final Uuid id = Uuid.parse(string); + + assertNotNull(id); + assertEquals(id.id(), 0xFFFFFFFF); + } }