bind variables and constraint functions

[bwallis42]

should this work?

{code}
Query query = queryManager.createQuery("SELECT d.* FROM [inf:document] AS d WHERE ISDESCENDANTNODE(d, $path) LIMIT 1", Query.JCR_SQL2);
query.bindValue("path", vf.createValue("/ns:123456");
{code}

I get a parse error for the SQL

{code}
au.com.infomedix.cpf4.cu.exceptions.Cpf4Exception: Unexpected JCR repository error
          at au.com.infomedix.cpf4.ds.PatientRecordOperations.deletePatient(PatientRecordOperations.java:510)
          at au.com.infomedix.cpf4.ds.PatientRecordOperationsTest.testDeletePatient(PatientRecordOperationsTest.java:374)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at java.lang.reflect.Method.invoke(Method.java:597)
          at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
          at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
          at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
          at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
          at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
          at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)
          at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
          at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)
          at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)
          at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
          at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
          at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
          at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
          at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
          at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
          at org.junit.runners.ParentRunner.run(ParentRunner.java:300)
          at org.mockito.internal.runners.JUnit45AndHigherRunnerImpl.run(JUnit45AndHigherRunnerImpl.java:37)
          at org.mockito.runners.MockitoJUnitRunner.run(MockitoJUnitRunner.java:62)
          at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
          at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
          at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
          at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
          at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
          at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
Caused by: javax.jcr.query.InvalidQueryException: The JCR-SQL2 query "SELECT d.* FROM [inf:document] AS d WHERE ISDESCENDANTNODE(d, $path) LIMIT 1" is not well-formed: Expecting ')' but found 'path' at line 1, column 64: SDESCENDANTNODE(d, $ ===>> path) LIMIT 1
          at org.modeshape.jcr.JcrQueryManager.createQuery(JcrQueryManager.java:149)
          at org.modeshape.jcr.JcrQueryManager.createQuery(JcrQueryManager.java:104)
          at au.com.infomedix.cpf4.ds.PatientRecordOperations.deletePatient(PatientRecordOperations.java:489)
          ... 29 more
 {code}

If I hardcode the path into the isdescendant() constraint then the query works as expected.

[rhauch]

No, the JSR-283 specification and its JCR-SQL2 grammar require that the second parameter of the "ISDESCENDANTNODE" constraint must be a path. See our documentation for details.

Feel free to log an enhancement request.

[bwallis42]

Unless it is standard I'm not sure I would want to use it unless there was a really good reason to.

With SQL there is often a good reason to use a prepared statement for efficiency reasons. Does this apply with modeshape and JCR_SQL2 with bind variables? If so would there be some performance advantage to using bind variables in ISDESCENDANTNODE constraints? And if there is a potential for a performance benefit then I might want to use a non standard feature (and hopefully get it pushed into the standard in a later release).

thanks.

[rhauch]

With SQL there is often a good reason to use a prepared statement for efficiency reasons. Does this apply with modeshape and JCR_SQL2 with bind variables?

This may change in the future so that we don't re-plan a query. But as of today, there is no such performance reason to use bind variables with JCR-SQL2. It is merely a convenience so that you don't have to build a whole String with a new query via substitution, prior to submitting your query.

One other reason people often use prepared statements with JDBC is to prevent SQL Injection., which is when the user supplies parameters that are not checked and simply put into the query string. One way this works is that supplied parameters include characters that terminate the existing query, start a second query or update statement, and start a comment (which results in the rest of the application's query string being ignored.

There are a number of reasons why SQL Injection is far, far less of a problem:

1. All ModeShape query languages are read-only, which means that **if** SQL Injection were possible it would never modify any data. **If** SQL Injection were possible, then the worst thing a successful attacker might achieve is reading more data than the query should normally return.
2. Nodes returned by queries must be accessible by the session issuing the query, so a successful attacker would never be able to see anything the session doesn't allow them to see.
3. ModeShape query languages do not support comments. There's no way to "ignore" the rest of the query.
4. Full-text search queries are converted by ModeShape into a JCR-SQL2 query where the supplied full-text search expression is used as a *value* in a CONTAINS function. (Any quotes or characters in the search expression that are used in JCR-SQL2 are escaped, meaning a search expression cannot terminate the CONTAINS functiona and merely add other JCR-SQL2 constraints; if an attacker does this, then they're just searching for text containing their additional query constraints.)

However, SQL Injection might still be possible if developers don't think. Do not simply take user-supplied values and simply concatenate these values with your own strings to build a query. If you are going to take user-supplied data, perform a validation check (e.g., verify it is a number, a date, an email address, etc.) before building the query.

But if you do take user-supplied values, are unable to properly validate the values, and use them in queries, the best way to avoid a SQL Injection attack is to use the QOM approach. This way you will force the user-supplied values into the correct parts of the query, so that when ModeShape builds the query plan it will match the query you build and will only use the user-supplied values as you expect.

ModeShape is no magic bullet. Developers must still use good practices when building applications.

[bwallis42]

MODE-1781

thanks.