Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ RFC ] Formatting qualified names with %N (like %C, but allow intermediate dots) #14

Open
lexidor opened this issue Jul 29, 2021 · 1 comment

Comments

@lexidor
Copy link

lexidor commented Jul 29, 2021

This RFC depends on hhvm's AsyncMysqlConnection->queryAsync() method.

The function appendColumnTableName() in mysql_client/Query.cpp does a runtime check on the QueryArgument passed. If it is a string, it places backticks around it and escapes any actual backticks by tossing in an extra backtick. If a twoTuple is passed, each element of the tuple is escaped this way and they are concatenated using a .. The threeTuple variant interprets the third element as an alias.
Hhvm and Hack have a concept of a tuple(string, string). These tuple types do not match twoTuple and threeTuple. This makes this logic unreachable from hhvm. This means that %C can never be transformed into

`tablename`.`columnname` or `tablename`.`columnname` AS `alias`

Passing the string "t.c"` to %C renders as

`t.c` instead of `t`.`c`

This means you can not pass a qualified name, since the database parser trips over qualified names that have the dot in the backticked context. I do not remiss the aliased threeTuple, but not having a way to passing a qualified name hurts. Hack code can use %T.%C and pass two string arguments. Code that intends to operate with both qualified and unqualified names has to fall back to using %Q.

I would like to be able to use a formatting specifier with works with both kinds of names. This could be performed by tossing in a backtick before and after a . in a string. Adding this to %C will transform a query which fails to parse into one that parses. This may not be what we want if callers expected untrusted arguments to not be able to reach ambiguous fields (join queries with overlapping columns). It also breaks native CPP consumers which pass tuples. Can we introduce a new specifier (I was thinking %N for name) which only operates on strings and tosses in backticks around dots? We might want to throw if the string has more than two parts to prevent injections of database level qualified names (dbname.tablename.columnname).

@fredemmott
Copy link
Contributor

Hhvm and Hack have a concept of a tuple(string, string). These tuple types do not match twoTuple and threeTuple. This makes this logic unreachable from hhvm.

It could be done as far as runtime is concerned by special-casing 2/3-element arrays in squangle, but this isn't possible in a way that's compatible with Hack's type system, so adding a new placeholder as suggested is the only way to do this without making hack code less safe.

@lexidor lexidor changed the title [ RFC ] Formatting qualified names in %T and %C [ RFC ] Formatting qualified names with %N (like %C, but allow intermediate dots) Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants