Skip to content

Add a feature to allow leading plus sign (JsonReadFeature.ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS) #774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Jun 23, 2022
Merged

Add a feature to allow leading plus sign (JsonReadFeature.ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS) #774

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2f87986
support option to enable trailing decimal point
pjfanning Jun 21, 2022
1ae2f52
Update FastParserNonStandardNumberParsingTest.java
pjfanning Jun 21, 2022
0bda2ca
remove deprecated code
pjfanning Jun 21, 2022
bb13d11
init work to support leading plus sign
pjfanning Jun 21, 2022
d978b86
add tests
pjfanning Jun 21, 2022
ccb40e1
Update NonStandardNumberParsingTest.java
pjfanning Jun 21, 2022
22df350
Merge remote-tracking branch 'upstream/2.14' into leading-plus-sign
pjfanning Jun 22, 2022
76905f5
Merge branch '2.14' into leading-plus-sign
pjfanning Jun 22, 2022
3aec651
Merge remote-tracking branch 'upstream/2.14' into leading-plus-sign
pjfanning Jun 22, 2022
3dae271
avoid recursion (because that approach allows multiple leading +)
pjfanning Jun 22, 2022
b221055
Update ReaderBasedJsonParser.java
pjfanning Jun 22, 2022
5a61dfb
avoid recursion
pjfanning Jun 22, 2022
d0c5805
try to fix infinity tests
pjfanning Jun 22, 2022
95d3a58
Update UTF8DataInputJsonParser.java
pjfanning Jun 22, 2022
405a03c
Update NonStandardNumberParsingTest.java
pjfanning Jun 22, 2022
2fba4df
extra test case
pjfanning Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/JsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ public enum Feature {
@Deprecated
ALLOW_NUMERIC_LEADING_ZEROS(false),

/**
* @deprecated Use {@link com.fasterxml.jackson.core.json.JsonReadFeature#ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS} instead
*/
ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS(false),

/**
* @deprecated Use {@link com.fasterxml.jackson.core.json.JsonReadFeature#ALLOW_LEADING_DECIMAL_POINT_FOR_NUMBERS} instead
*/
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/json/JsonReadFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,21 @@ public enum JsonReadFeature
@SuppressWarnings("deprecation")
ALLOW_LEADING_ZEROS_FOR_NUMBERS(false, JsonParser.Feature.ALLOW_NUMERIC_LEADING_ZEROS),

/**
* Feature that determines whether parser will allow
* JSON decimal numbers to start with a plus sign
* (like: +123). If enabled, no exception is thrown, and the number
* is parsed as though a leading sign had not been present.
*<p>
* Since JSON specification does not allow leading plus signs,
* this is a non-standard feature, and as such disabled by default.
*
* @since 2.14
*/
@SuppressWarnings("deprecation")
ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS(false, JsonParser.Feature.ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS),


/**
* Feature that determines whether parser will allow
* JSON decimal numbers to start with a decimal point
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,10 @@ public final JsonToken nextToken() throws IOException

// Ok: we must have a value... what is it?

if (i == '+' && isEnabled(JsonReadFeature.ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS.mappedFeature())) {
return nextToken();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this is problematic as it allows something like ++++4 I think. Or, potentially, DoS with 10,000 plus signs. :)

But I think it should be possible to handle this as a new entry in switch below, maybe adding case right above 0 ... 9, reading next character, falling through?
Same goes for all parsers.

}

JsonToken t;

switch (i) {
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/json/UTF8DataInputJsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,10 @@ public JsonToken nextToken() throws IOException
return _currToken;
}

if (i == INT_PLUS && isEnabled(JsonReadFeature.ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS.mappedFeature())) {
return nextToken();
}

// Nope: do we then expect a comma?
if (_parsingContext.expectComma()) {
if (i != INT_COMMA) {
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,10 @@ public JsonToken nextToken() throws IOException
return (_currToken = JsonToken.END_OBJECT);
}

if (i == INT_PLUS && isEnabled(JsonReadFeature.ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS.mappedFeature())) {
return nextToken();
}

// Nope: do we then expect a comma?
if (_parsingContext.expectComma()) {
if (i != INT_COMMA) {
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/fasterxml/jackson/core/read/FastParserNonStandardNumberParsingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public class FastParserNonStandardNumberParsingTest
{
private final JsonFactory fastFactory =
JsonFactory.builder()
.enable(JsonReadFeature.ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS)
.enable(JsonReadFeature.ALLOW_LEADING_DECIMAL_POINT_FOR_NUMBERS)
.enable(JsonReadFeature.ALLOW_TRAILING_DECIMAL_POINT_FOR_NUMBERS)
.enable(StreamReadFeature.USE_FAST_DOUBLE_PARSER)
Expand Down
76 changes: 64 additions & 12 deletions src/test/java/com/fasterxml/jackson/core/read/NonStandardNumberParsingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ public class NonStandardNumberParsingTest
extends com.fasterxml.jackson.core.BaseTest
{
private final JsonFactory JSON_F = JsonFactory.builder()
.enable(JsonReadFeature.ALLOW_LEADING_PLUS_SIGN_FOR_NUMBERS)
.enable(JsonReadFeature.ALLOW_LEADING_DECIMAL_POINT_FOR_NUMBERS)
.enable(JsonReadFeature.ALLOW_TRAILING_DECIMAL_POINT_FOR_NUMBERS)
.build();
Expand All @@ -31,6 +32,22 @@ public void testLeadingDotInDecimal() throws Exception {
}
}

/*
* The format "+NNN" (as opposed to "NNN") is not valid JSON, so this should fail
*/
public void testLeadingPlusSignInDecimal() throws Exception {
for (int mode : ALL_MODES) {
JsonParser p = createParser(mode, " +123 ");
try {
p.nextToken();
fail("Should not pass");
} catch (JsonParseException e) {
verifyException(e, "expected digit (0-9) to follow minus sign, for valid numeric value");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cowtowncoder I've left this in draft because the existing code throws an exception with a confusing message if a plus sign precedes a number - is it ok if I use this PR to change the message to say that plus signs are not allowed in front of JSON numbers?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, absolutely, improvements to error messages are good.

}
p.close();
}
}

/**
* The format "NNN." (as opposed to "NNN") is not valid JSON, so this should fail
*/
Expand Down Expand Up @@ -73,23 +90,58 @@ public void testTrailingDotInDecimalAllowedReader() throws Exception {
_testTrailingDotInDecimalAllowed(jsonFactory(), MODE_READER);
}

public void testLeadingPlusSignInDecimalAllowedAsync() throws Exception {
_testLeadingPlusSignInDecimalAllowed(jsonFactory(), MODE_DATA_INPUT);
}

public void testLeadingPlusSignInDecimalAllowedBytes() throws Exception {
_testLeadingPlusSignInDecimalAllowed(jsonFactory(), MODE_INPUT_STREAM);
_testLeadingPlusSignInDecimalAllowed(jsonFactory(), MODE_INPUT_STREAM_THROTTLED);
}

public void testLeadingPlusSignInDecimalAllowedReader() throws Exception {
_testLeadingPlusSignInDecimalAllowed(jsonFactory(), MODE_READER);
}

private void _testLeadingDotInDecimalAllowed(JsonFactory f, int mode) throws Exception
{
JsonParser p = createParser(f, mode, " .125 ");
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
assertEquals(0.125, p.getValueAsDouble());
assertEquals("0.125", p.getDecimalValue().toString());
assertEquals(".125", p.getText());
p.close();
try (JsonParser p = createParser(f, mode, " .125 ")) {
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
assertEquals(0.125, p.getValueAsDouble());
assertEquals("0.125", p.getDecimalValue().toString());
assertEquals(".125", p.getText());
}
}

private void _testLeadingPlusSignInDecimalAllowed(JsonFactory f, int mode) throws Exception
{
try (JsonParser p = createParser(f, mode, " +125 ")) {
assertEquals(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(125.0, p.getValueAsDouble());
assertEquals("125", p.getDecimalValue().toString());
assertEquals("125", p.getText());
}
try (JsonParser p = createParser(f, mode, " +.125 ")) {
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
assertEquals(0.125, p.getValueAsDouble());
assertEquals("0.125", p.getDecimalValue().toString());
assertEquals(".125", p.getText());
}
}

private void _testTrailingDotInDecimalAllowed(JsonFactory f, int mode) throws Exception
{
JsonParser p = createParser(f, mode, " 125. ");
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
assertEquals(125.0, p.getValueAsDouble());
assertEquals("125", p.getDecimalValue().toString());
assertEquals("125.", p.getText());
p.close();
try (JsonParser p = createParser(f, mode, " 125. ")) {
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
assertEquals(125.0, p.getValueAsDouble());
assertEquals("125", p.getDecimalValue().toString());
assertEquals("125.", p.getText());
}
try (JsonParser p = createParser(f, mode, " 125. ")) {
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
assertEquals(125.0, p.getValueAsDouble());
assertEquals("125", p.getDecimalValue().toString());
assertEquals("125.", p.getText());
}
}
}