Skip to content

Commit f6f29d5

Browse files
dmaccormackdmaccormack
committed
feat: Ignore extraneous ampersands for wfu-implied-composite values
1 parent 6066dc0 commit f6f29d5

File tree

4 files changed

+211
-13
lines changed

4 files changed

+211
-13
lines changed

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,43 @@ static final class Parse { //NOPMD - internal utility class
6666
private Parse() {
6767
}
6868

69+
/**
70+
* Used to skip/ignore "extra" ampersands in www-form-urlencoded data.
71+
* This skipping/ignoring is done in order to be compatible with
72+
* existing decoders (e.g. those built into browsers and web
73+
* service/application containers), which also ignore "extra"
74+
* ampersands rather than inserting empty values or throwing an
75+
* Error/Exception.
76+
*
77+
* <p>Note, this skipping/ignoring is <b>only</b> for
78+
* wfu-value-separator (ampersand), <b>not</b> value-separator
79+
* (comma). This is on purpose. There's no compatibility concern
80+
* in the later case and it allows for the option to use a zero
81+
* length string as the empty string.
82+
*/
83+
static int skipAmps(CharSequence text, int off, int end) {
84+
if (off == end || text.charAt(off) != '&') {
85+
// nothing to skip
86+
return off;
87+
}
88+
89+
int pos;
90+
91+
for (pos = off; pos < end && text.charAt(pos) == '&'; pos++) {
92+
// skip all consecutive amps
93+
}
94+
95+
if (pos == end) {
96+
// one or more amps followed by end-of-string
97+
return end;
98+
}
99+
100+
//
101+
// return the position of the last amp before the next usable char
102+
//
103+
return pos - 1;
104+
}
105+
69106
static NumberBuilder newNumberBuilder(
70107
ValueFactory<?,?,?,?,?,?,?,?,?,?> factory) {
71108

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

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -691,11 +691,28 @@ private <R> R parse(
691691

692692
final boolean wwwFormUrlEncoded = this.isFormUrlEncoded();
693693

694+
//
695+
// when I'm effectively a replacement for
696+
// ServletRequest.getParameterValues() or similar then I need to
697+
// accept and skip extra ampersands.
698+
//
699+
final boolean skipExtraAmps = wwwFormUrlEncoded
700+
&& (impliedArray || impliedObject);
701+
694702
int parseDepth = 1;
695703
int parseValueCount = 0;
696704

705+
if (skipExtraAmps) {
706+
//
707+
// ignore random amps at the beginning of the string
708+
//
709+
while (pos < stop && text.charAt(pos) == WFU_VALUE_SEPARATOR) {
710+
pos++;
711+
}
712+
}
713+
697714
for (;;) {
698-
if (pos == stop) {
715+
if (stop <= pos) {
699716
throw new SyntaxException(MSG_STILL_OPEN, pos);
700717
}
701718

@@ -781,15 +798,21 @@ private <R> R parse(
781798
stateStack.pop();
782799
result.setLocation(pos).addEmptyComposite();
783800

784-
if (pos == stop && parseDepth == 1) {
785-
if (impliedArray) {
786-
return result.addArrayElement().endArray().getResult();
787-
}
788-
if (impliedObject) {
789-
return result.addObjectElement().endObject().getResult();
801+
if (parseDepth == 1) { // NOPMD - AvoidLiteralsInIfCondition
802+
if (skipExtraAmps) {
803+
pos = JsonUrl.Parse.skipAmps(text, pos, stop);
790804
}
791805

792-
throw new SyntaxException(MSG_STILL_OPEN, pos);
806+
if (pos == stop) {
807+
if (impliedArray) {
808+
return result.addArrayElement().endArray().getResult();
809+
}
810+
if (impliedObject) {
811+
return result.addObjectElement().endObject().getResult();
812+
}
813+
814+
throw new SyntaxException(MSG_STILL_OPEN, pos);
815+
}
793816
}
794817

795818
continue;
@@ -814,6 +837,8 @@ private <R> R parse(
814837

815838
pos += litlen;
816839

840+
int litend = pos; // NOPMD = capture literal end position
841+
817842
if (pos == stop) {
818843
throw new SyntaxException(MSG_STILL_OPEN, pos);
819844
}
@@ -825,6 +850,10 @@ private <R> R parse(
825850
if (!wwwFormUrlEncoded || parseDepth != 1) {
826851
throw new SyntaxException(MSG_BAD_CHAR, pos);
827852
}
853+
//
854+
// not calling JsonUrl.Parse.skipAmps here because I
855+
// can't find a use case where it is needed
856+
//
828857
// fall through
829858
case VALUE_SEPARATOR:
830859
//
@@ -852,7 +881,7 @@ private <R> R parse(
852881
result
853882
.setLocation(litpos)
854883
.beginArray()
855-
.addLiteral(text, litpos, pos);
884+
.addLiteral(text, litpos, litend);
856885

857886
continue;
858887

@@ -887,6 +916,9 @@ private <R> R parse(
887916
throw new SyntaxException(MSG_EXTRA_CHARS, pos);
888917

889918
case 1:
919+
if (skipExtraAmps) {
920+
pos = JsonUrl.Parse.skipAmps(text, pos, stop);
921+
}
890922
if (pos == stop) {
891923
if (impliedArray) {
892924
return result
@@ -976,6 +1008,10 @@ private <R> R parse(
9761008
.setLocation(litpos)
9771009
.addLiteral(text, litpos, pos);
9781010

1011+
if (skipExtraAmps && parseDepth == 1) {
1012+
pos = JsonUrl.Parse.skipAmps(text, pos, stop);
1013+
}
1014+
9791015
if (pos == stop) {
9801016
if (parseDepth == 1 && impliedArray) {
9811017
return result.addArrayElement().endArray().getResult();
@@ -993,6 +1029,11 @@ private <R> R parse(
9931029
if (!wwwFormUrlEncoded || parseDepth != 1) {
9941030
throw new SyntaxException(MSG_BAD_CHAR, pos);
9951031
}
1032+
//
1033+
// not calling JsonUrl.Parse.skipAmps here because I
1034+
// can't find a use case where it is needed
1035+
//
1036+
9961037
// fall through
9971038
case VALUE_SEPARATOR:
9981039
stateStack.set(0, State.IN_ARRAY);
@@ -1013,6 +1054,9 @@ private <R> R parse(
10131054
throw new SyntaxException(MSG_EXTRA_CHARS, pos);
10141055

10151056
case 1:
1057+
if (skipExtraAmps) {
1058+
pos = JsonUrl.Parse.skipAmps(text, pos, stop);
1059+
}
10161060
if (pos == stop) {
10171061
if (impliedArray) {
10181062
return result.addArrayElement().endArray().getResult();
@@ -1066,7 +1110,11 @@ private <R> R parse(
10661110
result
10671111
.setLocation(litpos)
10681112
.addLiteral(text, litpos, pos);
1069-
1113+
1114+
if (skipExtraAmps && parseDepth == 1) {
1115+
pos = JsonUrl.Parse.skipAmps(text, pos, stop);
1116+
}
1117+
10701118
if (pos == stop) {
10711119
if (parseDepth == 1 && impliedObject) {
10721120
return result.addObjectElement().endObject().getResult();
@@ -1105,6 +1153,9 @@ private <R> R parse(
11051153
throw new SyntaxException(MSG_EXTRA_CHARS, pos);
11061154

11071155
case 1:
1156+
if (skipExtraAmps) {
1157+
pos = JsonUrl.Parse.skipAmps(text, pos, stop);
1158+
}
11081159
if (pos == stop) {
11091160
if (impliedArray) {
11101161
return result.addArrayElement().endArray().getResult();
@@ -1131,6 +1182,11 @@ private <R> R parse(
11311182
litpos = pos;
11321183
litlen = parseLiteralLength(text, pos, stop, MSG_STILL_OPEN);
11331184
pos += litlen;
1185+
litend = pos;
1186+
1187+
if (skipExtraAmps && parseDepth == 1) {
1188+
pos = JsonUrl.Parse.skipAmps(text, pos, stop);
1189+
}
11341190

11351191
if (pos == stop) {
11361192
if (impliedObject && parseDepth == 1) {
@@ -1139,7 +1195,7 @@ private <R> R parse(
11391195
.addMissingValue(
11401196
text,
11411197
litpos,
1142-
pos)
1198+
litend)
11431199
.addObjectElement()
11441200
.endObject()
11451201
.getResult();
@@ -1174,7 +1230,7 @@ private <R> R parse(
11741230
.addMissingValue(
11751231
text,
11761232
litpos,
1177-
pos);
1233+
litend);
11781234

11791235
stateStack.set(0, State.OBJECT_AFTER_ELEMENT);
11801236
continue;
@@ -1188,7 +1244,7 @@ private <R> R parse(
11881244

11891245
result
11901246
.setLocation(litpos)
1191-
.addObjectKey(text, litpos, pos);
1247+
.addObjectKey(text, litpos, litend);
11921248

11931249
pos++;
11941250
continue;

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

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,6 +1400,50 @@ void testObjectWfu(String text) {
14001400
factory.newObjectBuilder()));
14011401
}
14021402

1403+
@ParameterizedTest
1404+
@Tag(TAG_PARSE)
1405+
@Tag(TAG_OBJECT)
1406+
@ValueSource(strings = {
1407+
"a=b&&c:d",
1408+
"a=b&c=d&",
1409+
"a=b&c=d&&",
1410+
"a=b&&c=d",
1411+
"a&&c=d",
1412+
"a=b&&c",
1413+
"a=b&&c&",
1414+
"&a=b&c=d",
1415+
"a=b&&c:d&&e:f",
1416+
})
1417+
void testObjectWfuExtraAmps(String text) {
1418+
Parser p = new Parser();
1419+
1420+
assertThrows(
1421+
SyntaxException.class,
1422+
() -> p.parseObject(
1423+
text,
1424+
factory,
1425+
factory.newObjectBuilder()),
1426+
text);
1427+
1428+
p.setFormUrlEncoded(true);
1429+
1430+
assertTrue(p.isFormUrlEncoded(), text);
1431+
1432+
assertObjectWfu(
1433+
text,
1434+
p.parseObject(
1435+
text,
1436+
factory,
1437+
factory.newObjectBuilder(),
1438+
(key, pos) -> {
1439+
switch (key) {
1440+
case "a": return factory.getString("b");
1441+
case "c": return factory.getString("d");
1442+
default: return null;
1443+
}
1444+
}));
1445+
}
1446+
14031447
private void assertArrayWfu(String text, A actual) {
14041448
assertEquals(
14051449
factory.getString("a"),
@@ -1445,6 +1489,49 @@ void testArrayWfu(String text) {
14451489
factory.newArrayBuilder()));
14461490
}
14471491

1492+
@ParameterizedTest
1493+
@Tag(TAG_PARSE)
1494+
@Tag(TAG_ARRAY)
1495+
@ValueSource(strings = {
1496+
"&a",
1497+
"a&",
1498+
"&a&",
1499+
"a&&b",
1500+
"&a&b",
1501+
"&a&b&",
1502+
"a&b&",
1503+
"a&b&&c&d",
1504+
"a&b&c&&d",
1505+
"a&&false&&1.234&&null&&()&&(1)&&(1,2,3)&&(a:d,b:a)&&",
1506+
"a&true&",
1507+
"a&()&",
1508+
"a&(1)&",
1509+
"a&(1,2)&",
1510+
"a&(a:b)&",
1511+
})
1512+
void testArrayWfuExtraAmps(String text) {
1513+
Parser p = new Parser();
1514+
1515+
assertThrows(
1516+
SyntaxException.class,
1517+
() -> p.parseArray(
1518+
text,
1519+
factory,
1520+
factory.newArrayBuilder()),
1521+
text);
1522+
1523+
p.setFormUrlEncoded(true);
1524+
1525+
assertTrue(p.isFormUrlEncoded(), text);
1526+
1527+
assertArrayWfu(
1528+
text,
1529+
p.parseArray(
1530+
text,
1531+
factory,
1532+
factory.newArrayBuilder()));
1533+
}
1534+
14481535
@ParameterizedTest
14491536
@Tag(TAG_PARSE)
14501537
@Tag(TAG_ARRAY)

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,4 +135,22 @@ public boolean isSkipNulls() {
135135
assertTrue(options.isSkipNulls(), test);
136136
}
137137

138+
@Test
139+
void testFormUrlEncoded() {
140+
String test = "FormUrlEncoded";
141+
142+
assertFalse(JsonUrlOptions.isFormUrlEncoded(null), test);
143+
assertFalse(new JsonUrlOptions() {}.isFormUrlEncoded(), test);
144+
145+
JsonUrlOptions options = new JsonUrlOptions() {
146+
@Override
147+
public boolean isFormUrlEncoded() {
148+
return true;
149+
}
150+
};
151+
152+
assertTrue(JsonUrlOptions.isFormUrlEncoded(options), test);
153+
assertTrue(options.isFormUrlEncoded(), test);
154+
}
155+
138156
}

0 commit comments

Comments
 (0)