alekseyl/niceql

Modifies whitespace within sql strings

eki opened this issue · 6 comments

eki commented

Newlines and extra whitespace are collapsed into single spaces within sql string literals. When running INSERT or UPDATE statements through Niceql - as with the pg_adapter_with_nicesql option - can lead to data corruption.

Example:

[2] pry(main)> Niceql::Prettifier.prettify_sql("select 'foo\nbar  baz'")
=> "select \e[0;36;49m'foo bar baz'\e[0m"

I think the bug is likely here: https://github.com/alekseyl/niceql/blob/master/lib/niceql.rb#L129

I apologize that I don't have time to contribute a bug fix PR.

Without a fix, I don't think pg_adapter_with_nicesql can be used safely even in development.

alekseyl commented

hm, missed this issue.

Acknowledged. Don't have ETA though, but at least now I know that there is an issue )

  • 👀2
apurvis commented

The README says:

Please pay attention: untill issue #16 is resolved any UPDATE or INSERT request might corrupt your data, don't use on production!

Is this true even if pg_adapter_with_nicesql is turned off?

In other words: is it the case that any usage / installation of this gem within a rails application is inherently unsafe, or is it only unsafe if you configure it certain ways?

alekseyl commented

Is this true even if pg_adapter_with_nicesql is turned off?

nope, its not. It's not touching SQL before execution unless specified.

In other words: is it the case that any usage / installation of this gem within a rails application is inherently unsafe,

By default its disabled.

or is it only unsafe if you configure it certain ways?

Yes you can configure it temporarily to work in an unsafe way, to debug ceratin query, but that is not default configuraiton, and there is a WARNING about that.

alekseyl commented

Anyway, I'm started to think about removing this setting completely. It's too way dangerous if enabled in prod. Even though I'm constantly pointing out about that, even if I would fix bug, still too way dangerous to be used for all queries...

alekseyl commented

Good news everyone, finally I've managed to spend some noticeable amount of time and completely refactored core logic piece, which will bring the dollarsigned literals and also will keep literals as is: #21

Fix is ready to merge, I'll take a look with a fresh eyem, definitely will change some naming, and then it will be released.

alekseyl commented

fixed in 0.6 ver, fill free to check!

  • 🎉2