jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.54k stars 4.02k forks source link

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

Closed rjbgaspar closed 2 years ago

rjbgaspar commented 2 years ago

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.

DanielFran commented 2 years ago

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

DanielFran commented 2 years ago

Adding a bug bounty since this is a bug

rjbgaspar commented 2 years ago

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.

atomfrede commented 2 years ago

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

atomfrede commented 2 years ago

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