Replace usage of "addslashes" with prepared statements#16708
Replace usage of "addslashes" with prepared statements#16708tianon wants to merge 1 commit intonextcloud:masterfrom tianon:addslashes
Conversation
|
I guess |
:pass should be in single quotes (check the examples here: https://www.postgresql.org/docs/9.6/sql-createrole.html). |
lib/private/Setup/PostgreSQL.php
Outdated
| // create the user | ||
| $query = $connection->prepare("CREATE USER " . addslashes($this->dbUser) . " CREATEDB PASSWORD '" . addslashes($this->dbPassword) . "'"); | ||
| $query->execute(); | ||
| $query = $connection->prepare('CREATE USER :user CREATEDB PASSWORD :pass'); |
There was a problem hiding this comment.
| $query = $connection->prepare('CREATE USER :user CREATEDB PASSWORD :pass'); | |
| $query = $connection->prepare('CREATE USER :user CREATEDB PASSWORD \':pass\''); |
or with double quotes for the sql statment and single quotes for pass.
There was a problem hiding this comment.
It's a prepared statement, so the quotes are added automatically.
I believe the issue is that the username is getting quoted like a normal string but actually requires a different type of quoting.
There was a problem hiding this comment.
It's a prepared statement, so the quotes are added automatically.
Right ;)
There was a problem hiding this comment.
I think https://stackoverflow.com/a/18901826/433558 is the answer; I'll switch :user out for pg_escape_identifier as soon as I get a chance.
|
Ref #13318 |
|
@icewind1991 as you use postgres reguraly |
|
Hello @icewind1991 ;) |
|
PIng! |
|
Could anyone else help out here if @icewind1991 is not available? @nextcloud/server-triage |
|
what's the state here? Should it be moved to 23? |
|
It really would benefit from someone more well-versed in both nextcloud and
postgresql taking over and figuring out the full proper fix.
|
|
/rebase |
…_literal Signed-off-by: Tianon Gravi <admwiggin@gmail.com>
|
Tests are failing |
skjnldsv
added
2. developing
|
Yeah, this was (I guess vainly) hoping someone else would care enough to take over and figure out what's going wrong and how to fix it correctly -- I'm going to close for now, but if anyone else wants to pick this back up, feel free to start with my commit if it's helpful! |
8 participants
As referenced in #15187 (comment), #11311, #1260.
I figured opening a PR was a better (concrete) way to open a discussion around this. 😄 ❤️
I'm not actually a user of nextcloud, so I'm not 100% sure what to do to test this effectively, but I'm willing to give it a shot (although it appears there might be CI that does so already?).