Add SpEL support for table and column names

[funky-eyes]

We're from the [Seata] https://github.com/seata/seata community, we're a highly available, high-performance distributed transaction component, and currently our server side is considering moving from using jdbc to spring-data-r2dbc, but we've found some issues
Our transaction information is stored in mysql, oracle and other databases, because we support the resource isolation of transactions by configuring different table names, and then assembling them into corresponding sqls to use, which makes us unable to use @table to set a fixed table name, perhaps we can set the right through environment.resolvePlaceholders() or open doSelect table
If this requirement is reasonable, I am willing to complete this job

[schauder]

I'm highly sceptical of this since it seems likely to open us up to SQL injection.

https://xkcd.com/327/

[funky-eyes]

I'm highly sceptical of this since it seems likely to open us up to SQL injection.

https://xkcd.com/327/

I don't think there is an SQL injection problem with @table("${dynamic_table:user_01})
Environment is determined by the application developer and there should be no possibility of SQL injection

[schauder]

Environment is determined by the application developer

That is exactly the problem. A developer can use any variable and sooner than later will use untrusted data. And since bind variables can't be used, it has to be done by String concatenation, which pretty much the definition of a SQL injection problem.

[funky-eyes]

If it is because the environment variable is set by the developer, then the @table can also be set by the developer and becomes a point of existence for sql injection.
Is there a greater risk associated with methods like update or delete intable?
@schauder

[schauder]

If it is because the environment variable is set by the developer, then the https://github.com/table can also be set by the developer and becomes a point of existence for sql injection.

No, the Table annotation is safe because its values have to be compile time constants. Therefore they are trusted values by definition. This is not about the developer doing something malicious. It is about untrusted data (input from the user, data from a database ...) ending up as part of a sql statement.

[funky-eyes]

If it is because the environment variable is set by the developer, then the https://github.com/table can also be set by the developer and becomes a point of existence for sql injection.

No, the Table annotation is safe because its values have to be compile time constants. Therefore they are trusted values by definition. This is not about the developer doing something malicious. It is about untrusted data (input from the user, data from a database ...) ending up as part of a sql statement.

I still don't quite understand, even the parameters of properties, env, -D are set by the developer, who will attack their own application? I just wish this feature could be more flexible?

[funky-eyes]

Assuming I have multiple shards in my database, how do I aggregate the results of select * from table1 and select * from table2 when I use spring-data-r2dbc?

[funky-eyes]

unless I go to use DatabaseClient

[funky-eyes]

for example
java -jar -DTABLE=abc app.jar or export TABLE=abc java -jar app.jar

@table("${TABLE:cba}")

[schauder]

We came up with this plan:

Have SpEL support for names provided in @Table and @Column annotations but run the results through a sanitizer.
The sanitizer will be a bean implementing a simple interface and the default implementation out of the box by Spring Data will be rather restrictive. Something like only accept digits, ASCII letters and _. Stuff that doesn't get accepted will throw an exception.

If someone really wants to allow strange names they can do so be providing a custom sanitizer. Of course it is then up to them to make sure the whole is still safe.

[schauder]

The feature should also apply to reference and key columns specified via @MappedCollection

[funky-eyes]

@schauder

Hi，How long before the feature is expected to be available

[schauder]

We don't have schedule for this. And to be honest I don't consider it high priority.

Of course if there would be a good pull request ...

[funky-eyes]

We don't have schedule for this. And to be honest I don't consider it high priority.

Of course if there would be a good pull request ...

@schauder @mp911de
Can I overload the following method.
createPersistentEntity(TypeInformation,NamingStrategy)->createPersistentEntity(TypeInformation,NamingStrategy,ApplicationContext)
or
public JdbcMappingContext(NamingStrategy namingStrategy) -> public JdbcMappingContext(NamingStrategy namingStrategy,ApplicationContext)

This gives me a way to pass the ApplicationContext into the RelationalPersistentEntityImpl for support of spel expressions
I'd be happy to submit a relevant pull request

[schauder]

I'm not sure what method you want to overload, my IDE doesn't find any with that signature.

[funky-eyes]

I'm not sure what method you want to overload, my IDE doesn't find any with that signature.

@schauder
https://github.com/spring-projects/spring-data-relational/blob/main/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/JdbcMappingContext.java
public JdbcMappingContext(NamingStrategy namingStrategy) -> public JdbcMappingContext(NamingStrategy namingStrategy,ApplicationContext)
or
https://github.com/spring-projects/spring-data-commons/blob/main/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java
createPersistentEntity

[schauder]

I don't think we want to overload that. PersistentEntity and PersistentProperty are static objects, in the sense that they are created once in the lifetime of an application and then don't change.
It feels off to have it reference a context that may include request scoped variables.
I therefor would prefer their methods that provide column/table names to take a SpelQueryContext.

@mp911de your thoughts on this?

[funky-eyes]

@schauder
Maybe replace ApplicationContext with Environment, holding it just to resolveRequiredPlaceholders for tableName and schemaName in @Table when creating PersistentEntity

[funky-eyes]

Maybe MappingContext would be a good choice too