Skip to content

Commit 193b621

Browse files
dmaccormackdmaccormack
committed
fix: Remove JsonUrlStringBuilder.add(char) method
The JsonUrlStringBuilder.add(char) was simply appending the given character to the buffer without proper encoding. It was only used by JsonUrlWriter.write(char[]), but it's public so it could be used externally. The add(char) method was removed and replaced with addCodePoint(int), which properly handles structural characters and UNICODE.
1 parent f6f29d5 commit 193b621

File tree

6 files changed

+218
-36
lines changed

6 files changed

+218
-36
lines changed

‎module/jsonurl-core/src/main/java/org/jsonurl/JsonTextBuilder.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ public interface JsonTextBuilder<A,R> {
7070
*/
7171
JsonTextBuilder<A,R> addNull() throws IOException;
7272

73+
/**
74+
* Add a UNICODE codepoint.
75+
*/
76+
JsonTextBuilder<A,R> addCodePoint(int codepoint) throws IOException;
77+
7378
/**
7479
* Add a long value.
7580
*/
@@ -95,11 +100,6 @@ public interface JsonTextBuilder<A,R> {
95100
*/
96101
JsonTextBuilder<A,R> add(boolean value) throws IOException;
97102

98-
/**
99-
* Add a boolean value.
100-
*/
101-
JsonTextBuilder<A,R> add(char value) throws IOException;
102-
103103
/**
104104
* Add a string value.
105105
* @param text a valid CharSequence

‎module/jsonurl-core/src/main/java/org/jsonurl/JsonUrl.java‎

Lines changed: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -568,11 +568,42 @@ private static StringEncoding getStringEncoding(
568568
}
569569
}
570570

571+
@SuppressWarnings({
572+
"PMD.AvoidLiteralsInIfCondition",
573+
})
574+
private static void encode(
575+
Appendable dest,
576+
int codePoint,
577+
String[] hexEncode,
578+
int position) throws IOException {
579+
580+
if (codePoint < 0x80) {
581+
dest.append(hexEncode[codePoint]);
582+
583+
} else if (codePoint < 0x800) {
584+
dest.append(hexEncode[0xC0 | (codePoint >> 6)]);
585+
dest.append(hexEncode[0x80 | (codePoint & 0x3F)]);
586+
587+
} else if (codePoint < 0x10000) {
588+
dest.append(hexEncode[0xE0 | (codePoint >> 12)]);
589+
dest.append(hexEncode[0x80 | ((codePoint >> 6) & 0x3F)]);
590+
dest.append(hexEncode[0x80 | (codePoint & 0x3F)]);
591+
592+
} else if (codePoint < 0x200000) {
593+
dest.append(hexEncode[0xF0 | (codePoint >> 18)]);
594+
dest.append(hexEncode[0x80 | ((codePoint >> 12) & 0x3F)]);
595+
dest.append(hexEncode[0x80 | ((codePoint >> 6) & 0x3F)]);
596+
dest.append(hexEncode[0x80 | (codePoint & 0x3F)]);
597+
598+
} else {
599+
throw new MalformedInputException(position);
600+
}
601+
}
602+
571603
/**
572604
* Hex encode as UTF-8 .
573605
*/
574606
@SuppressWarnings({
575-
"PMD.AvoidLiteralsInIfCondition",
576607
"PMD.AvoidReassigningParameters", // needed for edge case
577608
"PMD.CyclomaticComplexity", // yup, encoding UTF-8 branches a lot
578609
"PMD.ModifiedCyclomaticComplexity",
@@ -628,28 +659,8 @@ private static void encode(
628659
} else {
629660
cp = c;
630661
}
631-
632-
if (cp < 0x80) {
633-
dest.append(hexEncode[cp]);
634-
635-
} else if (cp < 0x800) {
636-
dest.append(hexEncode[0xC0 | (cp >> 6)]);
637-
dest.append(hexEncode[0x80 | (cp & 0x3F)]);
638662

639-
} else if (cp < 0x10000) {
640-
dest.append(hexEncode[0xE0 | (cp >> 12)]);
641-
dest.append(hexEncode[0x80 | ((cp >> 6) & 0x3F)]);
642-
dest.append(hexEncode[0x80 | (cp & 0x3F)]);
643-
644-
} else if (cp < 0x200000) {
645-
dest.append(hexEncode[0xF0 | (cp >> 18)]);
646-
dest.append(hexEncode[0x80 | ((cp >> 12) & 0x3F)]);
647-
dest.append(hexEncode[0x80 | ((cp >> 6) & 0x3F)]);
648-
dest.append(hexEncode[0x80 | (cp & 0x3F)]);
649-
650-
} else {
651-
throw new MalformedInputException(i);
652-
}
663+
encode(dest, cp, hexEncode, i);
653664
}
654665
}
655666

@@ -1030,4 +1041,32 @@ public static <T extends Appendable> boolean appendLiteral(// NOPMD - Cyclomatic
10301041

10311042
return true;
10321043
}
1044+
1045+
/**
1046+
* Append the given UNICODE codepoint as a string literal.
1047+
* Note, this method effectively appends a string of length 1 and
1048+
* can't take advantage optimizations available to longer strings.
1049+
* It's primarily used for append character arrays. You don't
1050+
* want to call this in a loop as a means of appending a string.
1051+
* Use {@link #appendLiteral(Appendable, CharSequence, int, int, boolean, JsonUrlOptions)}
1052+
* for that.
1053+
*
1054+
* @param <T> destination type
1055+
* @param dest destination
1056+
* @param codePoint a UNICODE codePoint
1057+
* @param options a valid JsonUrlOptions or null
1058+
*/
1059+
public static <T extends Appendable> void appendCodePoint(
1060+
T dest,
1061+
int codePoint,
1062+
JsonUrlOptions options) throws IOException {
1063+
1064+
if (!isImpliedStringLiterals(options) && codePoint == APOS) {
1065+
dest.append("%27");
1066+
1067+
} else {
1068+
Encode.encode(dest, codePoint, CharUtil.HEXENCODE_UNQUOTED, 0);
1069+
}
1070+
}
1071+
10331072
}

‎module/jsonurl-core/src/main/java/org/jsonurl/JsonUrlTextAppender.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,12 @@ public JsonUrlTextAppender<A,R> addNull() throws IOException {
114114
return this;
115115
}
116116

117+
@Override
118+
public JsonUrlTextAppender<A,R> addCodePoint(int codePoint) throws IOException {
119+
JsonUrl.appendCodePoint(this, codePoint, this);
120+
return this;
121+
}
122+
117123
@Override
118124
public JsonUrlTextAppender<A,R> add(BigDecimal value) throws IOException {
119125
if (value == null) {
@@ -175,12 +181,6 @@ public JsonUrlTextAppender<A,R> add(boolean value) throws IOException {
175181
return this;
176182
}
177183

178-
@Override
179-
public JsonUrlTextAppender<A,R> add(char value) throws IOException {
180-
out.append(value);
181-
return this;
182-
}
183-
184184
@Override
185185
public JsonUrlTextAppender<A,R> add(
186186
CharSequence text,

‎module/jsonurl-core/src/main/java/org/jsonurl/j2se/JsonUrlWriter.java‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static org.jsonurl.JsonUrlOptions.isSkipNulls;
2323

2424
import java.io.IOException;
25+
import java.nio.charset.MalformedInputException;
2526
import java.util.Map;
2627
import org.jsonurl.JsonTextBuilder;
2728
import org.jsonurl.JsonUrlOptions;
@@ -366,7 +367,7 @@ public static <A,R> boolean write(
366367
* @param array null or a valid array
367368
* @return true if dest was modified
368369
*/
369-
public static <A,R> boolean write(
370+
public static <A,R> boolean write(// NOPMD - CyclomaticComplexity
370371
JsonTextBuilder<A,R> dest,
371372
JsonUrlOptions options,
372373
char... array) throws IOException {
@@ -376,12 +377,42 @@ public static <A,R> boolean write(
376377
}
377378

378379
dest.beginArray();
380+
381+
final int length = array.length;
379382

380-
for (int i = 0; i < array.length; i++) {
383+
for (int i = 0; i < length; i++) {
381384
if (i > 0) {
382385
dest.valueSeparator();
383386
}
384-
dest.add(array[i]);
387+
388+
char chr = array[i];
389+
390+
if (Character.isLowSurrogate(chr)) {
391+
throw new MalformedInputException(i);
392+
}
393+
394+
int codePoint;
395+
396+
if (Character.isHighSurrogate(chr)) {
397+
i++; // NOPMD - consumed two characters
398+
399+
if (i == length) {
400+
throw new MalformedInputException(i);
401+
}
402+
403+
char low = array[i];
404+
405+
if (Character.isHighSurrogate(low)) {
406+
throw new MalformedInputException(i);
407+
}
408+
409+
codePoint = Character.toCodePoint(chr, low);
410+
411+
} else {
412+
codePoint = chr;
413+
}
414+
415+
dest.addCodePoint(codePoint);
385416
}
386417

387418
dest.endArray();

‎module/jsonurl-core/src/test/java/org/jsonurl/JsonUrlStringBuilderTest.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.io.IOException;
2828
import java.math.BigDecimal;
2929
import java.math.BigInteger;
30+
import java.net.URLEncoder;
3031
import java.nio.charset.MalformedInputException;
3132
import org.junit.jupiter.api.Tag;
3233
import org.junit.jupiter.api.Test;
@@ -117,6 +118,63 @@ void testClear(String text) throws IOException {
117118
assertEquals(text, jup.clear().add(text).build(), text);
118119
}
119120

121+
private void assertCodePoint(
122+
String expected,
123+
int codePoint) throws IOException {
124+
assertCodePoint(expected, codePoint, false);
125+
}
126+
127+
private void assertCodePoint(
128+
String expected,
129+
int codePoint,
130+
boolean impliedStringLiteral) throws IOException {
131+
132+
JsonUrlStringBuilder jsb = new JsonUrlStringBuilder();
133+
jsb.setImpliedStringLiterals(impliedStringLiteral);
134+
String actual = jsb.clear().addCodePoint(codePoint).build();
135+
assertEquals(expected, actual, expected);
136+
}
137+
138+
@Test
139+
void testAddCodePointSingleQuote() throws IOException {
140+
assertCodePoint("%27", '\'', false);
141+
assertCodePoint("'", '\'', true);
142+
}
143+
144+
@Test
145+
void testAddCodePointSpace() throws IOException {
146+
assertCodePoint("+", ' ');
147+
}
148+
149+
@ParameterizedTest
150+
@ValueSource(chars = {
151+
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
152+
'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
153+
'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
154+
'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z',
155+
'0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
156+
'!', '-', '.', '_', '~', '!', '$', '*', '/', ';', '?', '@',
157+
})
158+
void testAddCodePointLiteral(char codePoint) throws IOException {
159+
String expected = String.valueOf(codePoint);
160+
String actual = new JsonUrlStringBuilder()
161+
.addCodePoint(codePoint)
162+
.build();
163+
164+
assertEquals(expected, actual, expected);
165+
}
166+
167+
@ParameterizedTest
168+
@ValueSource(chars = {
169+
'+', '\'', '(', ')', ',', ':'
170+
})
171+
void testAddCodePointEncoded(char codePoint) throws IOException {
172+
String expected = URLEncoder.encode(
173+
String.valueOf(codePoint), "UTF-8");
174+
175+
assertCodePoint(expected, codePoint);
176+
}
177+
120178
@Test
121179
void testAddNull() throws IOException {
122180
final String nullText = "null";
@@ -314,6 +372,21 @@ void testExceptionUtf8(String text) throws IOException {
314372
MalformedInputException.class,
315373
() -> new JsonUrlStringBuilder().add(text).build());
316374
}
375+
376+
@ParameterizedTest
377+
@Tag(TAG_EXCEPTION)
378+
@ValueSource(ints = {
379+
0x200000
380+
})
381+
void testAddCodePointException(int badCodePoint) throws IOException {
382+
assertThrows(
383+
MalformedInputException.class,
384+
() -> new JsonUrlStringBuilder().addCodePoint(badCodePoint));
385+
386+
assertThrows(
387+
MalformedInputException.class,
388+
() -> new JsonUrlStringBuilder().addCodePoint(badCodePoint));
389+
}
317390

318391
@Test
319392
@Tag(TAG_EXCEPTION)

‎module/jsonurl-core/src/test/java/org/jsonurl/j2se/JsonUrlWriterTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
import static org.junit.jupiter.api.Assertions.assertEquals;
2121
import static org.junit.jupiter.api.Assertions.assertFalse;
2222
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
import static org.junit.jupiter.api.Assertions.assertTrue;
2324

2425
import java.io.IOException;
26+
import java.nio.charset.MalformedInputException;
2527
import java.util.HashMap;
2628
import java.util.Map;
2729
import java.util.stream.Stream;
@@ -277,6 +279,43 @@ void testNullEnum() throws IOException {
277279
Enum.class.toString());
278280
}
279281

282+
@ParameterizedTest
283+
@MethodSource
284+
void testCharArray(ArrayTestCase test) throws IOException {
285+
JsonUrlStringBuilder jsb = new JsonUrlStringBuilder();
286+
assertTrue(JsonUrlWriter.write(jsb, test.values), test.expected);
287+
assertEquals(test.expected, jsb.build(), test.expected);
288+
}
289+
290+
static Stream<ArrayTestCase> testCharArray() {
291+
return Stream.of(
292+
new ArrayTestCase(
293+
"\u00A2".toCharArray(), // NOPMD - UNICODE escape
294+
"(%C2%A2)"),
295+
new ArrayTestCase(
296+
"\uD83C\uDF55".toCharArray(), // NOPMD - UNICODE escape
297+
"(%F0%9F%8D%95)")
298+
);
299+
}
300+
301+
@ParameterizedTest
302+
@MethodSource
303+
void testWriteCharArrayException(char... chars) throws IOException {
304+
assertThrows(
305+
MalformedInputException.class,
306+
() -> JsonUrlWriter.write(new JsonUrlStringBuilder(), chars));
307+
}
308+
309+
static Stream<char[]> testWriteCharArrayException() {
310+
return Stream.of(
311+
new char[] {Character.MIN_LOW_SURROGATE},
312+
new char[] {Character.MIN_HIGH_SURROGATE},
313+
new char[] {
314+
Character.MIN_HIGH_SURROGATE,
315+
Character.MIN_HIGH_SURROGATE}
316+
);
317+
}
318+
280319
@Test
281320
void testException() throws IOException {
282321
assertThrows(

0 commit comments

Comments
 (0)