Skip to content

Commit 3a98df1

Browse files
authored
Escape the ' character in the loggable method to help with log parser frameworks. (#3460)
The ' (single quote) character can confuse log parsers. This PR escapes this character when used within a log message. This change alters the serialized representation of the byte array as a string. Any usage of this representation would be therefore modified. Searching the codebase showed usage within Exception messages and logging, which is likely OK, but care should be taken to verify that no ill impact is exposed. Resolves #3461
1 parent 36af991 commit 3a98df1

File tree

3 files changed

+14
-8
lines changed

3 files changed

+14
-8
lines changed

fdb-extensions/src/main/java/com/apple/foundationdb/tuple/ByteArrayUtil2.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public class ByteArrayUtil2 {
3535

3636
private static final byte EQUALS_CHARACTER = (byte)'=';
3737
private static final byte DOUBLE_QUOTE_CHARACTER = (byte)'"';
38+
private static final byte SINGLE_QUOTE_CHARACTER = (byte)'\'';
3839
private static final byte BACKSLASH_CHARACTER = (byte)'\\';
3940
private static final byte MINIMUM_PRINTABLE_CHARACTER = 32;
4041
private static final int MAXIMUM_PRINTABLE_CHARACTER = 127;
@@ -71,7 +72,7 @@ public static String toHexString(@Nullable byte[] bytes) {
7172
* Creates a human-readable representation of {@code bytes} for logging purposes.
7273
* This differs from {@link ByteArrayUtil#printable(byte[])} in tha it excludes the
7374
* {@code =} and {@code "} characters, which can cause trouble when included as a
74-
* key or value in log messages with keys and values.
75+
* key or value in log messages with keys and values. For the same purpose, {@code '} is escaped as well.
7576
*
7677
* @param bytes a {@code byte} array
7778
* @return a hex representation of {@code bytes}
@@ -88,7 +89,7 @@ public static String loggable(@Nullable byte[] bytes) {
8889
for (byte b : bytes) {
8990
// remove '=' and '"' because they confuse parsing of key=value log messages
9091
if (b >= MINIMUM_PRINTABLE_CHARACTER && b < MAXIMUM_PRINTABLE_CHARACTER &&
91-
b != BACKSLASH_CHARACTER && b != EQUALS_CHARACTER && b != DOUBLE_QUOTE_CHARACTER) {
92+
b != BACKSLASH_CHARACTER && b != EQUALS_CHARACTER && b != DOUBLE_QUOTE_CHARACTER && b != SINGLE_QUOTE_CHARACTER) {
9293
sb.append((char)b);
9394
} else if (b == BACKSLASH_CHARACTER) {
9495
sb.append("\\\\");

fdb-extensions/src/test/java/com/apple/foundationdb/tuple/ByteArrayUtil2Test.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,14 @@ void testLoggable() {
4444
assertTrue(l.matches(hexRegex), l + " matches /" + hexRegex + "/");
4545
}
4646
for (int i = (byte)' '; i < (byte)'"'; i++) {
47-
assertEquals(Character.toString((char) i), ByteArrayUtil2.loggable(new byte[]{(byte)i}));
47+
assertEquals(Character.toString((char) i), ByteArrayUtil2.loggable(new byte[] {(byte)i}));
4848
}
4949
assertEquals("\\x22", ByteArrayUtil2.loggable(new byte[]{(byte)'"'}));
50-
for (int i = (byte)'"' + 1; i < (byte)'='; i++) {
50+
for (int i = (byte)'"' + 1; i < (byte)'\''; i++) {
51+
assertEquals(Character.toString((char) i), ByteArrayUtil2.loggable(new byte[]{(byte)i}));
52+
}
53+
assertEquals("\\x27", ByteArrayUtil2.loggable(new byte[]{(byte)'\''}));
54+
for (int i = (byte)'\'' + 1; i < (byte)'='; i++) {
5155
assertEquals(Character.toString((char) i), ByteArrayUtil2.loggable(new byte[]{(byte)i}));
5256
}
5357
assertEquals("\\x3d", ByteArrayUtil2.loggable(new byte[]{(byte)'='}));
@@ -91,8 +95,9 @@ void testRandomBytes(long seed) {
9195
assertArrayEquals(bytes, unlogged, "Unprinting loggable bytes should reconstruct original array");
9296
assertFalse(loggable.contains("="), "loggable string should not contain equals sign");
9397
assertFalse(loggable.contains("\""), "loggable string should not contain quote");
98+
assertFalse(loggable.contains("\'"), "loggable string should not contain single quote");
9499

95-
if (!printable.contains("=") && !printable.contains("\"")) {
100+
if (!printable.contains("=") && !printable.contains("\"") && !printable.contains("\'")) {
96101
assertEquals(printable, loggable);
97102
}
98103
}

fdb-extensions/src/test/java/com/apple/foundationdb/tuple/TupleTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public class TupleTest {
8989
.add(new ExpectedTupleEncoding<>(Double.MIN_NORMAL, "!\\x80\\x10\\x00\\x00\\x00\\x00\\x00\\x00"))
9090
.add(new ExpectedTupleEncoding<>(0.11577446748173348d, "!\\xbf\\xbd\\xa3e?\\x8b\\xbd\\x88"))
9191
// TODO: Add tests for Tuple encoding of BigIntegers (https://github.com/FoundationDB/fdb-record-layer/issues/18)
92-
.add(new ExpectedTupleEncoding<>(true, "'"))
92+
.add(new ExpectedTupleEncoding<>(true, "\\x27"))
9393
.add(new ExpectedTupleEncoding<>(false, "&"))
9494
.add(new ExpectedTupleEncoding<>(Versionstamp.complete(
9595
ByteArrayUtil2.unprint("\\x93\\x82\\x8db\\x97;\\xc5\\xfbt\\x9a"), 5180),
@@ -99,11 +99,11 @@ public class TupleTest {
9999
.add(new ExpectedTupleEncoding<>(new ArrayList<>(0), "\\x05\\x00"))
100100
.add(new ExpectedTupleEncoding<>(
101101
Arrays.asList(0, 1, "String " + zeroByteCharacter + " andZero", true, null, new byte[] {0, 0x03}),
102-
"\\x05\\x14\\x15\\x01\\x02String \\x00\\xff andZero\\x00'\\x00\\xff\\x01\\x00\\xff\\x03\\x00\\x00"))
102+
"\\x05\\x14\\x15\\x01\\x02String \\x00\\xff andZero\\x00\\x27\\x00\\xff\\x01\\x00\\xff\\x03\\x00\\x00"))
103103
.add(new ExpectedTupleEncoding<>(Tuple.from(), "\\x05\\x00"))
104104
.add(new ExpectedTupleEncoding<>(
105105
Tuple.fromList(Arrays.asList(0, 1, "String " + zeroByteCharacter + " andZero", true, null, new byte[] {0, 0x03})),
106-
"\\x05\\x14\\x15\\x01\\x02String \\x00\\xff andZero\\x00'\\x00\\xff\\x01\\x00\\xff\\x03\\x00\\x00"))
106+
"\\x05\\x14\\x15\\x01\\x02String \\x00\\xff andZero\\x00\\x27\\x00\\xff\\x01\\x00\\xff\\x03\\x00\\x00"))
107107
.add()
108108
.build();
109109

0 commit comments

Comments
 (0)