Ensure that new line chars don't break Panache projection #29851
Conversation
...me/src/main/java/io/quarkus/hibernate/orm/panache/common/runtime/CommonPanacheQueryImpl.java
Show resolved
Hide resolved
integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java
Outdated
Show resolved
Hide resolved
geoand
added
triage/waiting-for-ci
quarkus-bot
bot
added
kind/bugfix
and removed
triage/waiting-for-ci
| @@ -54,6 +54,8 @@ public void close() { | |||
|
|
|||
| private Map<String, Map<String, Object>> filters; | |||
|
|
|||
| private final String lineSeparator = System.getProperty("line.separator"); | |||
There was a problem hiding this comment.
Is it really a good idea to use this? From the original issue, it seems that the new lines are provided by the source code and this won't necessarily match the system line separator? Or am I missing something?
There was a problem hiding this comment.
Good point. Do we know how Java's multi line strings behave?
There was a problem hiding this comment.
This may be difficult to cover all use cases as someone may add “\n” , “\r”, or “\r\n” anywhere and request can comes from a config file.
Maybe it should be done for the 3 possible line separator, or using a "\R": https://docs.oracle.com/javase/8/docs/api/java/util/regex/Pattern.html#lineending
There was a problem hiding this comment.
I think that makes sense. Do you want to follow up with that @loicmathieu ?
There was a problem hiding this comment.
Well, given what we already do, I would just replace both \r and \n. If the user is using \n in strings embedded in the query, things are already broken so it won't make things worse to handle both.
There was a problem hiding this comment.
Thinking of weird contrived edge cases, what if someone writes a query like this...?
select o.id, 'came from Sydney' as comment from Orders o(it contains ' from ', though not as a sql keyword)
Fixes: #29838