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

Add @DisableQueriesWithoutBindParameters and @EnableQueriesWithoutBindParameters #3

Closed
jeanbisutti opened this issue Jun 23, 2019 · 10 comments
Assignees
Labels
✨ feature New feature or request sql SQL annotations

Comments

@jeanbisutti
Copy link
Collaborator

jeanbisutti commented Jun 23, 2019

Why

Using bind parameters is recommanded for performance. Moreover, bind parameters can prevent SQL injections.

References:

The role of @DisableQueriesWithoutBindParameters is to prevent the execution of requests without bind parameters. This annotation could be used whith a global scope, that is to say applied on each QuickPerf test.

@EnableQueriesWithoutBindParameters will cancel the behavior of @DisableQueriesWithoutBindParameters. @EnableQueriesWithoutBindParameters may be applied on a specific test method where some values can influence the execution plan (https://use-the-index-luke.com/sql/where-clause/bind-parameters).

Use cases

  • With @DisableQueriesWithoutBindParameters, a test sending the following request to database must Not fail:
 SELECT
        * 
    FROM
        book 
    WHERE
        isbn = ? 
        AND title = ?"], Params:[(978-0134685991,Effective Java)]

Java code example generating this request:

EntityManager em = emf.createEntityManager();
String sql = "SELECT * FROM book WHERE isbn = :isbn AND title = :title";
Query nativeQuery = em.createNativeQuery(sql)
                             .setParameter("isbn", "978-0321356680")
                             .setParameter("title", "Effective Java");
nativeQuery.getResultList();
  • With @DisableQueriesWithoutBindParameters, a test sending the following request to database must Not fail:
SELECT * FROM book 
  • With @DisableQueriesWithoutBindParameters, a test sending the following request to database must fail:
 SELECT
        * 
    FROM
        book 
    WHERE
        isbn = '978-0321356680' 
        AND title = 'Effective Java'

Java code example generating this request:

EntityManager em = emf.createEntityManager();
String sql = "SELECT * FROM book WHERE isbn = '978-0321356680' AND title = 'Effective Java'";
Query nativeQuery = em.createNativeQuery(sql);
nativeQuery.getResultList();
  • With @DisableQueriesWithoutBindParameters, a test sending the following request to database must Not fail:
UPDATE
        book 
    SET
        isbn = ?,
        title = ? 
    WHERE
        id = ?"], Params:[(978-0321356680,Effective Java,40)]

Java code example generating this request:

EntityManager em = emf.createEntityManager();
em.getTransaction().begin();
String sql = "UPDATE book SET isbn = :isbn, title = :title WHERE id = :id";
Query nativeQuery = em.createNativeQuery(sql)
                                  .setParameter("isbn", "978-0321356680")
                                  .setParameter("title", "Effective Java")
                                  .setParameter("id", 40);
nativeQuery.executeUpdate();
em.getTransaction().commit();
  • With @DisableQueriesWithoutBindParameters, a test sending the following request to database must fail:
 UPDATE
        book 
    SET
        isbn = '978-0321356680',
        title = 'Effective Java' 
    WHERE
        id = '40'

Java code example generating this request:

EntityManager em = emf.createEntityManager();
em.getTransaction().begin();
String sql = "UPDATE book SET isbn = '978-0321356680', title = 'Effective Java' WHERE id = '40'";
Query nativeQuery = em.createNativeQuery(sql);
nativeQuery.executeUpdate();
em.getTransaction().commit();
  • With @DisableQueriesWithoutBindParameters, a test sending the following request to database must Not fail:
    DELETE 
    FROM
        book 
    WHERE
        id = ?"], Params:[(40)

Java code example generating this request:

EntityManager em = emf.createEntityManager();
em.getTransaction().begin();
String sql = "DELETE FROM book WHERE id = :id";
Query nativeQuery = em.createNativeQuery(sql)
                                   .setParameter("id", 40);
nativeQuery.executeUpdate();
em.getTransaction().commit();
  • With @DisableQueriesWithoutBindParameters, a test sending the following request to database must fail:
 DELETE 
    FROM
        book 
    WHERE
        id = '40'

Java code example generating this request:

EntityManager em = emf.createEntityManager();
em.getTransaction().begin();
String sql = "DELETE FROM book WHERE id = '40'";
Query nativeQuery = em.createNativeQuery(sql);
nativeQuery.executeUpdate();
em.getTransaction().commit();

Implementation

This documentation can help you to implement, in particular this part.

@jeanbisutti jeanbisutti added ✨ feature New feature or request sql SQL annotations labels Jun 23, 2019
@jeanbisutti jeanbisutti changed the title Add @DisableQueriesAvoidingBindParameters and @EnableQueriesAvoidingBindParameters Add @DisableQueriesWithoutBindParameters and @EnableQueriesWithoutBindParameters Jul 4, 2019
@archyoshi
Copy link
Contributor

Hello, I want to work on this !

@jeanbisutti
Copy link
Collaborator Author

Great @archyoshi!

@jeanbisutti
Copy link
Collaborator Author

@archyoshi You can find here the steps to follow to add a SQL annotation to QuickPerf.

@jeanbisutti
Copy link
Collaborator Author

jeanbisutti commented Feb 20, 2020

This kind of code may help in SqlExecutions:

    /**
     * Examples :
     *  - "SET isbn = ?, title = ? " returns 2
     *  - "SET isbn = '123', title = '1 + 1 = 0' " returns 2
     */
    private long countUnquotedEquals(String setClause) {
        boolean inQuote = false;
        long equalCounter = 0;
        for (char c : setClause.toCharArray()) {
            if (c == '\'') {
               inQuote = !inQuote;
            }
            if (!inQuote && c == '=') {
                equalCounter++;
            }
        }
        return equalCounter;
    }

@nicokosi
Copy link

nicokosi commented Apr 2, 2020

Why

Using bind parameters is recommanded for performance. Moreover, bind parameters can prevent SQL injections.

References:

* _Oracle Performance Survival Guide: A Systematic Approach to Database Optimization_

* https://use-the-index-luke.com/sql/where-clause/bind-parameters

* https://dzone.com/articles/why-sql-bind-variables-are-important-for-performan

* https://blogs.oracle.com/sql/improve-sql-query-performance-by-using-bind-variables

* https://www.ibm.com/developerworks/library/se-bindvariables/index.html)

Thanks for these links. 👍

I like these tips from Use the index, Luke!:

Tip
Not using bind parameters is like recompiling a program every time.

Tip
In all reality, there are only a few cases in which the actual values affect the execution plan. You should therefore use bind parameters if in doubt—just to prevent SQL injections.

If I understand well, bind parameters are more likely to be security issues than performance issues.

@jeanbisutti
Copy link
Collaborator Author

jeanbisutti commented Apr 2, 2020

@nicokosi Thanks for your feedback.

Tip
In all reality, there are only a few cases in which the actual values affect the execution plan. You should therefore use bind parameters if in doubt—just to prevent SQL injections.

If I understand well, bind parameters are more likely to be security issues than performance issues.

I don't think that it is the meaning of the Use the index, Luke! paper:

[..]you should always use bind parameters except for values that shall influence the execution plan.
[...]
Tip
In all reality, there are only a few cases in which the actual values affect the execution plan. You should therefore use bind parameters if in doubt—just to prevent SQL injections.

In some cases, the values can influence the execution plan (and so we may have a better execution plan without bind parameters), however there are only a few cases according to Markus Winand. So you

should therefore use bind parameters if in doubt—just to prevent SQL injections

So, bind parameters are for both security and performance issues.

@jeanbisutti
Copy link
Collaborator Author

@nicokosi
The following papers clearly show that bind variables matter for performance :

@archyoshi
Copy link
Contributor

Hello !

The subject is advancing well, next steps to cover are related to some code refactorings as well as many test cases to add.

Refactoring
Compare bind parameters obtained by the method query.getParameters() with the 'supposed bind parameters' obtained either

  • by splitting the query on the "AND" keyword
    Be careful when the query uses an "IN" keyword !
    Be careful when the query contains an 'AND' word as the value of a given parameter !

  • by counting the number of = signs that we have
    Be careful when the query contains = as a value of a given parameter !

Remaining tests

  1. Test use case with 1 bind parameter and 1 unbin parameter (with different order)
  2. Test use case with OR keyword
  3. Test the other statements like INSERT/SELECT/DELETE/UPDATE...
  4. Test nested SELECT statements
  5. Tester SELECT statement using LIKE keyword
  6. Tester SELECT statement using IN
  7. Tester SELECT statement using BETWEEN
  8. Tester SELECT statement with keywords where, and, … using different cases (uppercase/lowercase)

@archyoshi
Copy link
Contributor

Hello, so here's a little update on what has been done so far and what remains:

  1. Test use case with 1 bind parameter and 1 unbind parameter (with different order)
  2. Test use case with OR keyword
  3. Test the other statements like INSERT/SELECT/DELETE/UPDATE...
  4. Test nested SELECT statements
  5. Tester SELECT statement using LIKE keyword
  6. Tester SELECT statement using IN
  7. Tester SELECT statement using BETWEEN
  8. Tester SELECT statement with keywords where, and, … using different cases (uppercase/lowercase)

We are working on this case:

  • Test with a statement with a tricky statement (the statement itself contains an sql keyword)

@jeanbisutti jeanbisutti added this to the 1.1 milestone Aug 3, 2020
@jeanbisutti
Copy link
Collaborator Author

Remaining cases (LIKE, IN, BETWEEN, ...) fixed with 81f61b7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature New feature or request sql SQL annotations
Projects
None yet
Development

No branches or pull requests

3 participants