4 ms·

Problem with that example is where do you get `safe`? Passing a template into `db.execute` lets the `db` instance handle safety specifically for the backend it'

Problem with that example is where do you get `safe`? Passing a template into `db.execute` lets the `db` instance handle safety specifically for the backend it's connected to. Otherwise, you'd need to create a `safe` function with a db connection to properly sanitize a string. And further, if `safe` just returns a string, you still lose out on the ability for `db.execute` to pass the parameter a different way -- you've lost the information that a variable is being interpolated into the string.

Tenoke 1y ago
db.safe same as the new db.execute with safety checks in it you create for the t-string but yes I can see some benefits (though I'm still not a fan for my own codebases so far) with using the values further or more complex cases than this.
- ubercore 1y ago
  Yeah but it would have to be something like `db.safe("SELECT * FROM table WHERE id = {}", row_id)` instead of `db.execute(t"SELECT * FROM table WHERE id = {row_id}")`. I'd prefer the second, myself.
  - Tenoke 1y ago
    No, just `db.execute(f"QUERY WHERE name = {db.safe(name)}")` And you add the safety inside db.safe explicitly instead of implicitly in db.execute. If you want to be fancy you can also assign name to db.foos inside db.safe to use it later (even in execute).
    - ZiiS 1y ago
      But if someone omits the `safe` it may still work but allow injection.
      - thunky 1y ago
        Same is true if someone forgets to use t" and uses f" instead. At least db.safe says what it does, unlike t".
        ewidar 1y ago
        Not really, since f"" is a string and t"" is a template, you could make `db.execute` only accept templates, maybe have `db.execute(Template)` and `db.unsafeExecute(str)`
        thunky 1y ago
        agreed. but then you're breaking the existing `db.execute(str)`. if you don't do that, and instead add `db.safe_execute(tpl: Template)`, then you're back to the risk that a user can forget to call the safe function. also, you're trusting that the library implementer raises a runtime exception if a string a passed where a template is expected. it's not enough to rely on type-checks/linting. and there is probably going to be a temptation to accept `db.execute(sql: Union[str, Template])` because this is non-breaking, and sql without params doesn't need to be templated - so it's breaking some stuff that doesn't need to be broken. i'm not saying templates aren't a good step forward, just that they're also susceptible to the same problems we have now if not used correctly.
        ubercore 1y ago
        Then make `db.unsafe_execute` take a string.
        thunky 1y ago
        Yeah, you could. I'm just saying that by doing this you're breaking `db.execute` by not allowing it to take it string like it does now. Libraries may not want to add a breaking change for this.
        fwip 1y ago
        Your linter can flag the type mismatch, and/or the function can reject f"" at runtime. This is because t"" yields a Template, not a str. Template is also more powerful/concise in that the stringify function can handle the "formatting" args however it looks. Note also, that there's no requirement that the template ever become a str to be used.
    - sanderjd 1y ago
      This is just extra boilerplate though, for what purpose?. I think one thing you might be missing is that in the t-string version, `db.execute` is not taking a string; a t-string resolves to an object of a particular type. So it is doing your `db.safe` operation, but automatically.
    - panzi 1y ago
      Of course you can write code like that. This is about making it easier not to accidentally cause code injection by forgetting the call of safe(). JavaScript had the same feature and some SQL libraries allow only the passing of template strings, not normal strings, so you can't generate a string with code injection. If you have to dynamically generate queries they allow that a parameter is another template string and then those are merged correctly. It's about reducing the likelihood of making mistakes with fewer key strokes. We could all just write untyped assembly instead and could do it safely by paying really good attention.
    - quinnirill 1y ago
      What does db.safe do though? How does it know what is the safe way of escaping at that point of the SQL? It will have no idea whether it’s going inside a string, if it’s in a field name position, denotes a value or a table name. To illustrate the question further, consider a similar html.safe: f"<a href={html.safe(url)}>{html.safe(desc)</a>" - the two calls to html.safe require completely different escaping, how does it know which to apply?
  - Izkata 1y ago
    The first one already exists like: db.execute("SELECT * FROM table WHERE id = ?", (row_id,))

Tenoke 1y ago

db.safe same as the new db.execute with safety checks in it you create for the t-string but yes I can see some benefits (though I'm still not a fan for my own codebases so far) with using the values further or more complex cases than this.

ubercore 1y ago
Yeah but it would have to be something like `db.safe("SELECT * FROM table WHERE id = {}", row_id)` instead of `db.execute(t"SELECT * FROM table WHERE id = {row_id}")`. I'd prefer the second, myself.
- Tenoke 1y ago
  No, just `db.execute(f"QUERY WHERE name = {db.safe(name)}")` And you add the safety inside db.safe explicitly instead of implicitly in db.execute. If you want to be fancy you can also assign name to db.foos inside db.safe to use it later (even in execute).
  - ZiiS 1y ago
    But if someone omits the `safe` it may still work but allow injection.
    - thunky 1y ago
      Same is true if someone forgets to use t" and uses f" instead. At least db.safe says what it does, unlike t".
      - ewidar 1y ago
        Not really, since f"" is a string and t"" is a template, you could make `db.execute` only accept templates, maybe have `db.execute(Template)` and `db.unsafeExecute(str)`
        thunky 1y ago
        agreed. but then you're breaking the existing `db.execute(str)`. if you don't do that, and instead add `db.safe_execute(tpl: Template)`, then you're back to the risk that a user can forget to call the safe function. also, you're trusting that the library implementer raises a runtime exception if a string a passed where a template is expected. it's not enough to rely on type-checks/linting. and there is probably going to be a temptation to accept `db.execute(sql: Union[str, Template])` because this is non-breaking, and sql without params doesn't need to be templated - so it's breaking some stuff that doesn't need to be broken. i'm not saying templates aren't a good step forward, just that they're also susceptible to the same problems we have now if not used correctly.
        ubercore 1y ago
        Then make `db.unsafe_execute` take a string.
        thunky 1y ago
        Yeah, you could. I'm just saying that by doing this you're breaking `db.execute` by not allowing it to take it string like it does now. Libraries may not want to add a breaking change for this.
      - fwip 1y ago
        Your linter can flag the type mismatch, and/or the function can reject f"" at runtime. This is because t"" yields a Template, not a str. Template is also more powerful/concise in that the stringify function can handle the "formatting" args however it looks. Note also, that there's no requirement that the template ever become a str to be used.
  - sanderjd 1y ago
    This is just extra boilerplate though, for what purpose?. I think one thing you might be missing is that in the t-string version, `db.execute` is not taking a string; a t-string resolves to an object of a particular type. So it is doing your `db.safe` operation, but automatically.
  - panzi 1y ago
    Of course you can write code like that. This is about making it easier not to accidentally cause code injection by forgetting the call of safe(). JavaScript had the same feature and some SQL libraries allow only the passing of template strings, not normal strings, so you can't generate a string with code injection. If you have to dynamically generate queries they allow that a parameter is another template string and then those are merged correctly. It's about reducing the likelihood of making mistakes with fewer key strokes. We could all just write untyped assembly instead and could do it safely by paying really good attention.
  - quinnirill 1y ago
    What does db.safe do though? How does it know what is the safe way of escaping at that point of the SQL? It will have no idea whether it’s going inside a string, if it’s in a field name position, denotes a value or a table name. To illustrate the question further, consider a similar html.safe: f"<a href={html.safe(url)}>{html.safe(desc)</a>" - the two calls to html.safe require completely different escaping, how does it know which to apply?
- Izkata 1y ago
  The first one already exists like: db.execute("SELECT * FROM table WHERE id = ?", (row_id,))