Skip to content
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.

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

LongValue does not catch NumberFormatException #208

Merged
merged 1 commit into from
Aug 1, 2016

Conversation

emopers
Copy link
Contributor

@emopers emopers commented Jan 12, 2016

LongValue.java calls java.lang.long.parseLong without first checking whether the argument parses. This lead to an uncaught NumberFormatException: Oracle Java 7 API specification.

This pull request adds a check with a more helpful exception message and also fix an indentation issue at line no. 38.

@wumpz
Copy link
Member

wumpz commented Jan 12, 2016

But were is the problem. The parser itself ensures that only long values are passed to LongValue. So do you have a problematic SQL, that produces a NumberFormatException?

@emopers emopers closed this Feb 5, 2016
@emopers
Copy link
Contributor Author

emopers commented Aug 1, 2016

Re-opening this pull request because, while JSQLParser may indeed ensure that only valid values are passed to the LongValue constructor, LongValue is a public class and its constructor is public. Thus, code that depends on JSQLParser without properly sanitizing their input could run into this problem.

For example, when the snippet below is run:

import net.sf.jsqlparser.expression.LongValue;

public class Test {
    public static void main(String[] args) {
        new LongValue("random");
    }
}

The exception would be thrown without any useful debugging information:

/tmp$ java -cp .:JSqlParser/target/jsqlparser-0.9.6-SNAPSHOT.jar Test
Exception in thread "main" java.lang.NumberFormatException: For input string: "random"
    at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
    at java.lang.Long.parseLong(Long.java:441)
    at java.lang.Long.parseLong(Long.java:483)
    at net.sf.jsqlparser.expression.LongValue.<init>(LongValue.java:37)
    at Test.main(Test.java:5)

@emopers emopers reopened this Aug 1, 2016
@coveralls
Copy link

coveralls commented Aug 1, 2016

Coverage Status

Coverage increased (+0.3%) to 83.771% when pulling 4fad4af on emopers:long_bad_76 into f5b515b on JSQLParser:master.

@wumpz
Copy link
Member

wumpz commented Aug 1, 2016

Agreed after JSqlParser is more and more used for SQL building purposes.

@wumpz wumpz merged commit cd6866c into JSQLParser:master Aug 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants