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

Bugfix/wild backslash attacks again in insert stmnt #259

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

michalkutrzeba-odrabiamy
Copy link

@michalkutrzeba-odrabiamy michalkutrzeba-odrabiamy commented Jan 25, 2023

Database: PostgreSQL 13.7

Replibyte assumed that ' can be escaped by \ and ', but it can only be escaped by ', so this statement select 'test \\' test'; assumed as correct in Replibyte is incorrect in reality.
And this select 'test test \'; assumed as incorrect is correct in reality.

select 'test '' test'; # ok
select 'test \\' test';
-- SQL:  select 'test \\' test'
-- unterminated quoted string at or near "'"
select 'test \' test';
-- SQL:  select 'test \' test'
-- unterminated quoted string at or near "'"

@michalq
Copy link

michalq commented Jan 25, 2023

Ok I see now that this utils.rs are common for mysql and psql, escaping that works in mysql doesn't work in psql and vice versa, this need to be reconsidered.

@evoxmusic
Copy link
Contributor

evoxmusic commented Jan 25, 2023

Hi guys, some tests are failing. Let me know if you need some help :) Thanks for your contribution

@michalq
Copy link

michalq commented Jan 27, 2023

Hi, thanks, I'll left this PR as is for now, but it needs to be done differently and slash escaping must be with different strategy for mysql and postgresql.

I found yet another problem with EOF when in dump script find more than 49 new lines, this seems to be very wrong, at least in my dump there are places with such new lines and dump ends importing too early, for now I commented it out, but I'm not sure if this is ok.

And yet another problem with file order, looks like read_dir returns files in totally random way, so I implemented sorting here.

Example:

Name: ./local-datastore/dump-1674753282544/13.dump
Name: ./local-datastore/dump-1674753282544/3.dump
Name: ./local-datastore/dump-1674753282544/25.dump
Name: ./local-datastore/dump-1674753282544/24.dump
Name: ./local-datastore/dump-1674753282544/2.dump
Name: ./local-datastore/dump-1674753282544/12.dump
Name: ./local-datastore/dump-1674753282544/19.dump
Name: ./local-datastore/dump-1674753282544/23.dump
Name: ./local-datastore/dump-1674753282544/9.dump
Name: ./local-datastore/dump-1674753282544/15.dump
Name: ./local-datastore/dump-1674753282544/5.dump
Name: ./local-datastore/dump-1674753282544/4.dump
Name: ./local-datastore/dump-1674753282544/14.dump
Name: ./local-datastore/dump-1674753282544/8.dump
Name: ./local-datastore/dump-1674753282544/22.dump
Name: ./local-datastore/dump-1674753282544/18.dump
Name: ./local-datastore/dump-1674753282544/21.dump
Name: ./local-datastore/dump-1674753282544/7.dump
Name: ./local-datastore/dump-1674753282544/17.dump
Name: ./local-datastore/dump-1674753282544/16.dump
Name: ./local-datastore/dump-1674753282544/6.dump
Name: ./local-datastore/dump-1674753282544/20.dump
Name: ./local-datastore/dump-1674753282544/1.dump
Name: ./local-datastore/dump-1674753282544/11.dump
Name: ./local-datastore/dump-1674753282544/10.dump

@evoxmusic
Copy link
Contributor

@michalq Do you think it's ready to be merged? (I didn't review it yet)

@michalq
Copy link

michalq commented Jan 31, 2023

@michalq Do you think it's ready to be merged? (I didn't review it yet)

It definitely should not be merged, it fixes escape behaviour for postgres but at the same time it breaks mysql since there is one method for both dbs, needs more work.

@evoxmusic evoxmusic changed the title Bugfix/wild backslash attacks again in insert stmnt Draft: Bugfix/wild backslash attacks again in insert stmnt Jan 31, 2023
@evoxmusic evoxmusic marked this pull request as draft January 31, 2023 16:56
@evoxmusic evoxmusic changed the title Draft: Bugfix/wild backslash attacks again in insert stmnt Bugfix/wild backslash attacks again in insert stmnt Jan 31, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants