From 47ddf477275ab70ee28e59f3f7c167fb21a2f117 Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Tue, 27 Aug 2024 14:03:51 -0400 Subject: [PATCH 1/8] Align identifier bytes correctly in ResourceLocator Adjust the starting index of System.arraycopy to correctly align the identifier bytes within the given array length. This change ensures that shorter identifiers are aligned at the end of the array, improving data consistency and correctness. --- .../io/opentdf/platform/sdk/nanotdf/ResourceLocator.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java index cf50e1fc..27224dd0 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java @@ -60,15 +60,15 @@ public ResourceLocator(final String resourceUrl, final String identifier) { } else if (identifierLen <= 2) { this.identifierType = NanoTDFType.IdentifierType.TWO_BYTES; this.identifier = new byte[NanoTDFType.IdentifierType.TWO_BYTES.getLength()]; - System.arraycopy(identifier.getBytes(), 0, this.identifier, 0, identifierLen); + System.arraycopy(identifier.getBytes(), 0, this.identifier, this.identifier.length - identifierLen, identifierLen); } else if (identifierLen <= 8) { this.identifierType = NanoTDFType.IdentifierType.EIGHT_BYTES; this.identifier = new byte[NanoTDFType.IdentifierType.EIGHT_BYTES.getLength()]; - System.arraycopy(identifier.getBytes(), 0, this.identifier, 0, identifierLen); + System.arraycopy(identifier.getBytes(), 0, this.identifier, this.identifier.length - identifierLen, identifierLen); } else if (identifierLen <= 32) { this.identifierType = NanoTDFType.IdentifierType.THIRTY_TWO_BYTES; this.identifier = new byte[NanoTDFType.IdentifierType.THIRTY_TWO_BYTES.getLength()]; - System.arraycopy(identifier.getBytes(), 0, this.identifier, 0, identifierLen); + System.arraycopy(identifier.getBytes(), 0, this.identifier, this.identifier.length - identifierLen, identifierLen); } else { throw new IllegalArgumentException("Unsupported identifier length: " + identifierLen); } From c019117817ce95f275f7df22f783b823ba5acc89 Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Tue, 27 Aug 2024 14:29:16 -0400 Subject: [PATCH 2/8] Trim whitespace from ResourceLocator identifier Previously, the returned identifier string could contain unnecessary padding. By trimming the identifier, we ensure it has no leading or trailing whitespace, improving consistency and preventing potential issues in downstream processing. --- .../java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java index 27224dd0..c84250a6 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java @@ -220,6 +220,7 @@ public String getIdentifierString() { actualLength = i + 1; } } - return new String(this.identifier, 0, actualLength); + String identifierPadded = new String(this.identifier, 0, actualLength); + return identifierPadded.trim(); } } \ No newline at end of file From d8419302f4abc0e6f56eaf38bc4c42951531d799 Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Tue, 10 Sep 2024 13:17:06 -0400 Subject: [PATCH 3/8] Fix array copy logic in ResourceLocator identifier Previously, identifiers were copied with an offset, leading to potential misalignment issues. The updated approach starts copying from the beginning of the destination array, ensuring correct placement of identifier bytes. This change improves the reliability and correctness of identifier handling. --- .../io/opentdf/platform/sdk/nanotdf/ResourceLocator.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java index c84250a6..bfbf0915 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java @@ -60,15 +60,15 @@ public ResourceLocator(final String resourceUrl, final String identifier) { } else if (identifierLen <= 2) { this.identifierType = NanoTDFType.IdentifierType.TWO_BYTES; this.identifier = new byte[NanoTDFType.IdentifierType.TWO_BYTES.getLength()]; - System.arraycopy(identifier.getBytes(), 0, this.identifier, this.identifier.length - identifierLen, identifierLen); + System.arraycopy(identifier.getBytes(), 0, this.identifier, 0, identifierLen); } else if (identifierLen <= 8) { this.identifierType = NanoTDFType.IdentifierType.EIGHT_BYTES; this.identifier = new byte[NanoTDFType.IdentifierType.EIGHT_BYTES.getLength()]; - System.arraycopy(identifier.getBytes(), 0, this.identifier, this.identifier.length - identifierLen, identifierLen); + System.arraycopy(identifier.getBytes(), 0, this.identifier, 0, identifierLen); } else if (identifierLen <= 32) { this.identifierType = NanoTDFType.IdentifierType.THIRTY_TWO_BYTES; this.identifier = new byte[NanoTDFType.IdentifierType.THIRTY_TWO_BYTES.getLength()]; - System.arraycopy(identifier.getBytes(), 0, this.identifier, this.identifier.length - identifierLen, identifierLen); + System.arraycopy(identifier.getBytes(), 0, this.identifier, 0, identifierLen); } else { throw new IllegalArgumentException("Unsupported identifier length: " + identifierLen); } From 9e631f5209ed40ae519a00bd664436b12a51401c Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Tue, 10 Sep 2024 13:33:48 -0400 Subject: [PATCH 4/8] Fix identifier nibble masking in ResourceLocator Corrected the masking operation for the identifier nibble to ensure it correctly processes the upper four bits of the byte. This fix aligns the masking logic with the intended handling of the protocolWithIdentifier byte. --- .../java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java index bfbf0915..910496e6 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java @@ -79,7 +79,7 @@ public ResourceLocator(ByteBuffer buffer) { // Get the first byte and mask it with 0xF to keep only the first four bits final byte protocolWithIdentifier = buffer.get(); int protocolNibble = protocolWithIdentifier & 0x0F; - int identifierNibble = (protocolWithIdentifier & 0xF0) >> 4; + int identifierNibble = ((protocolWithIdentifier & 0xFF) & 0xF0) >> 4; this.protocol = NanoTDFType.Protocol.values()[protocolNibble]; // body this.bodyLength = buffer.get(); From a4add25dc8a389e0549c3ee27a757e19ccb18255 Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Tue, 10 Sep 2024 13:45:50 -0400 Subject: [PATCH 5/8] Simplify identifierNibble calculation Removed unnecessary masking operation in the identifierNibble calculation. The new logic directly extracts the higher nibble by masking with 0xF0. --- .../java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java index 910496e6..cf07db6b 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java @@ -79,7 +79,7 @@ public ResourceLocator(ByteBuffer buffer) { // Get the first byte and mask it with 0xF to keep only the first four bits final byte protocolWithIdentifier = buffer.get(); int protocolNibble = protocolWithIdentifier & 0x0F; - int identifierNibble = ((protocolWithIdentifier & 0xFF) & 0xF0) >> 4; + int identifierNibble = protocolWithIdentifier & 0xF0; this.protocol = NanoTDFType.Protocol.values()[protocolNibble]; // body this.bodyLength = buffer.get(); From e9a34238b5431997db66940eeed215bb640926aa Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Tue, 10 Sep 2024 13:58:28 -0400 Subject: [PATCH 6/8] Fix identifierNibble calculation and improve protocol writing Corrected the calculation of identifierNibble to ensure accurate bitwise operations. Refactored the protocol writing logic to use a switch case, providing specific handling based on identifier types for better readability and maintainability. --- .../platform/sdk/nanotdf/ResourceLocator.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java index cf07db6b..740bab8f 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java @@ -79,7 +79,7 @@ public ResourceLocator(ByteBuffer buffer) { // Get the first byte and mask it with 0xF to keep only the first four bits final byte protocolWithIdentifier = buffer.get(); int protocolNibble = protocolWithIdentifier & 0x0F; - int identifierNibble = protocolWithIdentifier & 0xF0; + int identifierNibble = ((protocolWithIdentifier & 0xFF) & 0xF0) >> 4; this.protocol = NanoTDFType.Protocol.values()[protocolNibble]; // body this.bodyLength = buffer.get(); @@ -183,13 +183,21 @@ public int writeIntoBuffer(ByteBuffer buffer) { int totalBytesWritten = 0; // Write the protocol type. - if (identifierType == NanoTDFType.IdentifierType.NONE) { - buffer.put((byte) protocol.ordinal()); - totalBytesWritten += 1; // size of byte - } else { - buffer.put((byte) (identifierType.ordinal() << 4 | protocol.ordinal())); - totalBytesWritten += 1; + switch (identifierType) { + case NONE: + buffer.put((byte) protocol.ordinal()); + break; + case TWO_BYTES: + buffer.put((byte) ((0b0001 << 4) | protocol.ordinal())); + break; + case EIGHT_BYTES: + buffer.put((byte) ((0b0010 << 4) | protocol.ordinal())); + break; + case THIRTY_TWO_BYTES: + buffer.put((byte) ((0b0100 << 4) | protocol.ordinal())); + break; } + totalBytesWritten += 1; // size of byte // Write the url body length buffer.put((byte)bodyLength); From be8bfa6ae99e29e0f6ff88cbdc35c4898fc43727 Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Wed, 11 Sep 2024 13:09:19 -0400 Subject: [PATCH 7/8] Enhance ResourceLocatorTest with golden bytearray validation Updated `creatingResourceLocatorWithDifferentIdentifiers` to include validation against golden byte arrays for identifiers. Adjusted `provideUrlsAndIdentifiers` method to provide corresponding byte arrays for each identifier string. --- .../platform/sdk/nanotdf/ResourceLocatorTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/nanotdf/ResourceLocatorTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/nanotdf/ResourceLocatorTest.java index ec6a17c8..562a3475 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/nanotdf/ResourceLocatorTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/nanotdf/ResourceLocatorTest.java @@ -60,20 +60,22 @@ void writingResourceLocatorIntoBufferWithInsufficientSize() { @ParameterizedTest @MethodSource("provideUrlsAndIdentifiers") - void creatingResourceLocatorWithDifferentIdentifiers(String url, String identifier, int expectedLength) { + void creatingResourceLocatorWithDifferentIdentifiers(String url, String identifier, int expectedLength, byte[] goldenIdentifier) { locator = new ResourceLocator(url, identifier); assertEquals(url, locator.getResourceUrl()); assertEquals(identifier, locator.getIdentifierString()); + assertArrayEquals(goldenIdentifier, locator.getIdentifier()); assertEquals(expectedLength, locator.getIdentifier().length); } private static Stream provideUrlsAndIdentifiers() { return Stream.of( - Arguments.of("http://test.com", "F", 2), - Arguments.of("http://test.com", "e0", 2), - Arguments.of("http://test.com", "e0e0e0e0", 8), - Arguments.of("http://test.com", "e0e0e0e0e0e0e0e0", 32), - Arguments.of("https://test.com", "e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0",32 ) + Arguments.of("http://test.com", "F", 2, new byte[]{70, 0}), + Arguments.of("http://test.com", "e0", 2, new byte[]{101,48}), + Arguments.of("http://test.com", "12345", 8, new byte[]{49,50,51,52,53,0,0,0}), + Arguments.of("http://test.com", "e0e0e0e0", 8, new byte[]{101, 48, 101, 48, 101, 48, 101, 48}), + Arguments.of("http://test.com", "e0e0e0e0e0e0e0e0", 32, new byte[]{101,48,101,48,101,48,101,48,101,48,101,48,101,48,101,48,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}), + Arguments.of("https://test.com", "e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0",32, new byte[]{101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48, 101, 48}) ); } From 4f7892a2751b731e078ceb57cc5073249351521b Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Wed, 11 Sep 2024 14:53:24 -0400 Subject: [PATCH 8/8] Fix incorrect bitmask for THIRTY_TWO_BYTES case in ResourceLocator The bitmask for the THIRTY_TWO_BYTES case was incorrectly set to 0b0100, which has now been corrected to 0b0011. This ensures proper buffer allocation and protocol handling, preventing potential misbehavior. --- .../java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java index 740bab8f..ac189859 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/ResourceLocator.java @@ -194,7 +194,7 @@ public int writeIntoBuffer(ByteBuffer buffer) { buffer.put((byte) ((0b0010 << 4) | protocol.ordinal())); break; case THIRTY_TWO_BYTES: - buffer.put((byte) ((0b0100 << 4) | protocol.ordinal())); + buffer.put((byte) ((0b0011 << 4) | protocol.ordinal())); break; } totalBytesWritten += 1; // size of byte