generator-jhipster icon indicating copy to clipboard operation
generator-jhipster copied to clipboard

Bad SQL grammar is thrown for all entities with string ID types

Open rjbgaspar opened this issue 2 years ago • 4 comments

Hello everyone,

Overview of the issue

Due to the fix for SQL Injection in Reactive project included with Release 7.8.1 (2022-04-07) where jhipster has replaced the Criteria with Condition in the EntityManager bean. E.g.

Prior to version 7.8.1

public class EntityManager {
(...)	
    /**
     * Creates an SQL select statement from the given fragment and pagination parameters.
     * @param selectFrom a representation of a select statement.
     * @param entityType the entity type which holds the table name.
     * @param pageable page parameter, or null, if everything needs to be returned
     * @return sql select statement
     */
    public String createSelect(SelectFromAndJoin selectFrom, Class<?> entityType, Pageable pageable, Criteria criteria) {
        if (pageable != null) {
            if (criteria != null) {
                return createSelectImpl(
                        selectFrom.limitOffset(pageable.getPageSize(), pageable.getOffset()).where(Conditions.just(criteria.toString())),
                        entityType,
                        pageable.getSort()
                );
            } else {
                return createSelectImpl(
                        selectFrom.limitOffset(pageable.getPageSize(), pageable.getOffset()),
                        entityType,
                        pageable.getSort()
                );
            }
        } else {
            if (criteria != null) {
                return createSelectImpl(selectFrom.where(Conditions.just(criteria.toString())), entityType, null);
            } else {
                return createSelectImpl(selectFrom, entityType, null);
            }
        }
    }
(...)

And now

public class EntityManager {
(...)
    /**
     * Creates an SQL select statement from the given fragment and pagination parameters.
     * @param selectFrom a representation of a select statement.
     * @param entityType the entity type which holds the table name.
     * @param pageable page parameter, or null, if everything needs to be returned.
     * @param where condition or null. The condition to apply as where clause.
     * @return sql select statement
     */
    public String createSelect(SelectFromAndJoin selectFrom, Class<?> entityType, Pageable pageable, Condition where) {
        if (pageable != null) {
            if (where != null) {
                return createSelectImpl(
                    selectFrom.limitOffset(pageable.getPageSize(), pageable.getOffset()).where(where),
                    entityType,
                    pageable.getSort()
                );
            } else {
                return createSelectImpl(
                    selectFrom.limitOffset(pageable.getPageSize(), pageable.getOffset()),
                    entityType,
                    pageable.getSort()
                );
            }
        } else {
            if (where != null) {
                return createSelectImpl(selectFrom.where(where), entityType, null);
            } else {
                return createSelectImpl(selectFrom, entityType, null);
            }
        }
    }
(...)

the application will throw BadSqlGrammarException each time an entity is accessed by its ID.

Motivation for or Use Case

n.a.

Reproduce the error

Generate project with reactive with Spring WebFlux and set the database type to SQL

Related issues

SQL Injection in Reactive project

Suggest a Fix

We could generate ExampleRepositoryInternalImpl to wrap with single quote all entities with string type IDs, something like this:

class ExampleRepositoryInternalImpl extends...
(...)
    @Override
    public Mono<Example> findById(String id) {
        Comparison whereClause = Conditions.isEqual(entityTable.column("id"), Conditions.just("'" + id.toString() + "'"));
        return createQuery(null, whereClause).one();
    }
(...)

Despite the idea, we cannot forget that whenever we use the Condition API to filter a string type, we have to wrap the field with single quotes.

JHipster Version(s)

7.8.1 (2022-04-07)

JHipster configuration
reactive with Spring WebFlux? Yes
type of database? SQL
Entity configuration(s) entityName.json files generated in the .jhipster directory

JDL

entity Example {
    id String
    name String
}
Browsers and Operating System

n.a.

rjbgaspar avatar Jun 02 '22 08:06 rjbgaspar

Thanks @rjbgaspar to report the issue. Are you available to contribute with a PR?

DanielFran avatar Jun 02 '22 08:06 DanielFran

Adding a bug bounty since this is a bug

DanielFran avatar Jun 02 '22 08:06 DanielFran

Hi @DanielFran,

Maybe in the future, at the moment I don't know the implementation of the generator and it would take quite some time to do it.

When I have the time and knowledge, I intend to give my contribution.

rjbgaspar avatar Jun 02 '22 10:06 rjbgaspar

@DanielFran As I introduced this bug I can have a look to differentiate the id type and adapt the code.

atomfrede avatar Jun 13 '22 11:06 atomfrede

Going to work on this later today, so we could have it in the next 7.9 release @DanielFran

atomfrede avatar Aug 17 '22 11:08 atomfrede