diff --git a/.github/workflows/test-sql-escaping.yml b/.github/workflows/test-sql-escaping.yml new file mode 100644 index 0000000..17f78e7 --- /dev/null +++ b/.github/workflows/test-sql-escaping.yml @@ -0,0 +1,86 @@ +name: SQL Escaping Tests + +on: + push: + branches: [ fix/sql-escape-string-order ] + pull_request: + branches: [ main ] + +jobs: + unit-test: + name: Unit Tests (Free Pascal) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install Free Pascal + run: sudo apt-get update && sudo apt-get install -y fp-compiler + + - name: Compile tests + run: fpc Tests/TestSQLEscapeString.pas + + - name: Run tests + run: ./Tests/TestSQLEscapeString + + integration-test: + name: Integration Tests (Firebird + MariaDB) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Start Firebird + run: | + docker run -d --name firebird-test \ + -e FIREBIRD_DATABASE=testdb \ + -e FIREBIRD_USER=testuser \ + -e FIREBIRD_PASSWORD=testpass \ + -e ISC_PASSWORD=masterkey \ + jacobalberty/firebird:v4.0 + + - name: Start MariaDB + run: | + docker run -d --name mysql-test \ + -e MARIADB_ROOT_PASSWORD=testpass \ + -e MARIADB_DATABASE=testdb \ + mariadb:10 + + - name: Start SQL Server + run: | + docker run -d --name mssql-test \ + -e ACCEPT_EULA=Y \ + -e MSSQL_SA_PASSWORD='TestPass123!' \ + mcr.microsoft.com/mssql/server:2022-latest + + - name: Wait for databases + run: | + echo "Waiting for Firebird..." + for i in $(seq 1 30); do + if docker exec firebird-test /usr/local/firebird/bin/isql \ + -user SYSDBA -password masterkey localhost:/firebird/data/testdb \ + -i /dev/stdin <<< "SELECT 1 FROM RDB\$DATABASE;" > /dev/null 2>&1; then + echo "Firebird ready"; break + fi; sleep 2 + done + echo "Waiting for MariaDB..." + for i in $(seq 1 30); do + if docker exec mysql-test mariadb -uroot -ptestpass testdb \ + -e "SELECT 1;" > /dev/null 2>&1; then + echo "MariaDB ready"; break + fi; sleep 2 + done + echo "Waiting for SQL Server..." + for i in $(seq 1 30); do + if docker exec mssql-test /opt/mssql-tools18/bin/sqlcmd \ + -S localhost -U sa -P 'TestPass123!' -C \ + -Q "SELECT 1" > /dev/null 2>&1; then + echo "SQL Server ready"; break + fi; sleep 2 + done + + - name: Run integration tests + run: bash Tests/TestSQLEscapeIntegration.sh + + - name: Cleanup + if: always() + run: | + docker rm -f firebird-test mysql-test mssql-test 2>/dev/null || true diff --git a/DBTool/C_Database.pas b/DBTool/C_Database.pas index 297cdfb..8e64bed 100644 --- a/DBTool/C_Database.pas +++ b/DBTool/C_Database.pas @@ -2423,16 +2423,23 @@ function TDbToolDatabase.SQL_Escape_String(sString: String): String; result := StringReplace(result, '''', '''''', [rfReplaceAll]); end; - dtMySql, + dtMySql: + begin + // Important: escape backslashes first, then quotes, + // otherwise the backslash from \' gets double-escaped to \\' + result := StringReplace(result, '\', '\\', [rfReplaceAll]); + result := StringReplace(result, '''', '\''', [rfReplaceAll]); + end; + {$IFNDEF WIN64} - dtLocal, // Nicht getestet Unbekannt, ob es Escaping gibt, und wie dieses aussieht. + dtLocal, // BDE/Paradox/dBase: use double-quote escaping like SQL standard {$ENDIF} - dtInterbase, // Nicht getestet Unbekannt, ob es Escaping gibt, und wie dieses aussieht. - dtFirebird, // Nicht getestet Unbekannt, ob es Escaping gibt, und wie dieses aussieht. - dtAccess: // Nicht getestet. Unbekannt, ob es Escaping gibt, und wie dieses aussieht. + dtInterbase, + dtFirebird, + dtAccess: begin - result := StringReplace(result, '''', '\''', [rfReplaceAll]); - result := StringReplace(result, '\', '\\', [rfReplaceAll]); + // InterBase, Firebird and Access use SQL-standard '' escaping, not backslash + result := StringReplace(result, '''', '''''', [rfReplaceAll]); end; else raise Exception.Create('(TDbToolDatabase.SQL_Escape_String) ' + diff --git a/Tests/TestSQLEscapeIntegration.sh b/Tests/TestSQLEscapeIntegration.sh new file mode 100755 index 0000000..4b0a7ef --- /dev/null +++ b/Tests/TestSQLEscapeIntegration.sh @@ -0,0 +1,158 @@ +#!/bin/bash +# Integration test: verify SQL escaping against real databases +set -euo pipefail + +PASS=0 +FAIL=0 +SCRIPTDIR="$(cd "$(dirname "$0")" && pwd)" +TMPDIR=$(mktemp -d) + +assert_eq() { + local test_name="$1" expected="$2" actual="$3" + if [ "$expected" = "$actual" ]; then + echo "[PASS] $test_name" + ((PASS++)) || true + else + echo "[FAIL] $test_name" + echo " Expected: '$expected'" + echo " Actual: '$actual'" + ((FAIL++)) || true + fi +} + +assert_contains() { + local test_name="$1" needle="$2" haystack="$3" + if echo "$haystack" | grep -qi "$needle"; then + echo "[PASS] $test_name" + ((PASS++)) || true + else + echo "[FAIL] $test_name" + echo " Expected to contain: '$needle'" + echo " Actual: '$haystack'" + ((FAIL++)) || true + fi +} + +fb_exec() { + docker exec -i firebird-test /usr/local/firebird/bin/isql \ + -user SYSDBA -password masterkey localhost:/firebird/data/testdb 2>&1 +} + +my_file() { + # Copy SQL file into container and execute it + docker cp "$1" mysql-test:/tmp/query.sql + docker exec mysql-test mariadb -uroot -ptestpass testdb -sN -e "source /tmp/query.sql" 2>&1 +} + +echo "=== Firebird Integration Tests ===" +echo "Waiting for Firebird..." +for i in $(seq 1 30); do + if echo "SELECT 1 FROM RDB\$DATABASE;" | fb_exec > /dev/null 2>&1; then + echo "Firebird ready"; break + fi; sleep 2 +done + +# Setup +fb_exec << 'SQL' +CREATE TABLE test_escape (id INTEGER, val VARCHAR(200)); +COMMIT; +SQL + +# FB1: SQL-standard '' escaping +fb_exec << 'SQL' +INSERT INTO test_escape VALUES (1, 'O''Brien'); +COMMIT; +SQL +result=$(echo "SELECT val FROM test_escape WHERE id=1;" | fb_exec | tr -s ' ' | grep "O'" | sed "s/^ *//;s/ *$//") +assert_eq "Firebird: '' escaping produces O'Brien" "O'Brien" "$result" + +# FB2: Backslash literal (no escaping needed) +fb_exec << 'SQL' +INSERT INTO test_escape VALUES (2, 'C:\Users'); +COMMIT; +SQL +result=$(echo "SELECT val FROM test_escape WHERE id=2;" | fb_exec | tr -s ' ' | grep 'C:' | sed "s/^ *//;s/ *$//") +assert_eq "Firebird: backslash needs no escaping" 'C:\Users' "$result" + +# FB3: MySQL-style \' must fail on Firebird +fb3_result=$(echo "INSERT INTO test_escape VALUES (99, 'test\\'value'); COMMIT;" | fb_exec 2>&1 || true) +assert_contains "Firebird: backslash-quote rejected" "error\|statement\|unexpected" "$fb3_result" + +echo "" +echo "=== MariaDB Integration Tests ===" +echo "Waiting for MariaDB..." +for i in $(seq 1 30); do + if docker exec mysql-test mariadb -uroot -ptestpass testdb -e "SELECT 1;" > /dev/null 2>&1; then + echo "MariaDB ready"; break + fi; sleep 2 +done + +# Setup MariaDB +docker exec mysql-test mariadb -uroot -ptestpass testdb -e "CREATE TABLE test_escape (id INT, val VARCHAR(200));" + +# Use SQL files to avoid shell escaping issues +# MY1: \' works for quote escaping +cat > "$TMPDIR/my1.sql" << 'SQLEOF' +INSERT INTO test_escape VALUES (1, 'O\'Brien'); +SQLEOF +my_file "$TMPDIR/my1.sql" +result=$(docker exec mysql-test mariadb -uroot -ptestpass testdb -sN -e "SELECT val FROM test_escape WHERE id=1;") +assert_eq "MySQL: backslash-quote produces O'Brien" "O'Brien" "$result" + +# MY2: \\ needed for literal backslash (compare via HEX to avoid shell escaping) +cat > "$TMPDIR/my2.sql" << 'SQLEOF' +INSERT INTO test_escape VALUES (2, 'C:\\Users'); +SQLEOF +my_file "$TMPDIR/my2.sql" +result=$(docker exec mysql-test mariadb -uroot -ptestpass testdb -sN -e "SELECT HEX(val) FROM test_escape WHERE id=2;") +# C:\Users in hex = 433A5C5573657273 +assert_eq "MySQL: double-backslash produces C:\\Users" "433A5C5573657273" "$result" + +# MY3: Unescaped backslash is swallowed +cat > "$TMPDIR/my3.sql" << 'SQLEOF' +INSERT INTO test_escape VALUES (3, 'C:\Users'); +SQLEOF +my_file "$TMPDIR/my3.sql" +result=$(docker exec mysql-test mariadb -uroot -ptestpass testdb -sN -e "SELECT val FROM test_escape WHERE id=3;") +assert_eq "MySQL: unescaped backslash swallowed → C:Users" "C:Users" "$result" + +# MY4: The OLD bug — double escaping \\' produces wrong data +# Old code did: O'Brien → O\'Brien → O\\'Brien +# MySQL reads O\\' as: O\ (literal backslash) + ' (end of string) + Brien (syntax error) +cat > "$TMPDIR/my4.sql" << 'SQLEOF' +INSERT INTO test_escape VALUES (4, 'O\\'Brien'); +SQLEOF +my4_result=$(my_file "$TMPDIR/my4.sql" 2>&1 || true) +assert_contains "MySQL: double-escaped quote causes error (the bug)" "error\|syntax" "$my4_result" + +echo "" +echo "=== SQL Server: String Escaping ===" +echo "Waiting for SQL Server..." +SQLCMD="docker exec mssql-test /opt/mssql-tools18/bin/sqlcmd -S localhost -U sa -P TestPass123! -C -d tempdb" +for i in $(seq 1 30); do + if $SQLCMD -Q "SELECT 1" > /dev/null 2>&1; then + echo "SQL Server ready"; break + fi; sleep 2 +done + +$SQLCMD -Q "CREATE TABLE test_escape (id INT, val VARCHAR(200));" > /dev/null + +# MS1: '' escaping works (SQL standard) +$SQLCMD -Q "INSERT INTO test_escape VALUES (1, 'O''Brien');" > /dev/null +result=$($SQLCMD -h -1 -Q "SET NOCOUNT ON; SELECT val FROM test_escape WHERE id=1;" | sed 's/ *$//' | head -1) +assert_eq "SQL Server: '' escaping produces O'Brien" "O'Brien" "$result" + +# MS2: Backslash is literal (no escaping needed) +$SQLCMD -Q "INSERT INTO test_escape VALUES (2, 'C:\Users');" > /dev/null +result=$($SQLCMD -h -1 -Q "SET NOCOUNT ON; SELECT val FROM test_escape WHERE id=2;" | sed 's/ *$//' | head -1) +assert_eq "SQL Server: backslash is literal (no escaping)" "C:\Users" "$result" + +# MS3: \' does NOT work on SQL Server (not MySQL) +ms3_result=$($SQLCMD -Q "INSERT INTO test_escape VALUES (3, 'O\'Brien');" 2>&1 || true) +assert_contains "SQL Server: backslash-quote rejected" "syntax\|error\|Msg\|Unclosed" "$ms3_result" + +echo "" +echo "=== Results: $((PASS + FAIL)) tests, $PASS passed, $FAIL failed ===" +rm -rf "$TMPDIR" + +[ "$FAIL" -eq 0 ] diff --git a/Tests/TestSQLEscapeString.pas b/Tests/TestSQLEscapeString.pas new file mode 100644 index 0000000..5f0fbe0 --- /dev/null +++ b/Tests/TestSQLEscapeString.pas @@ -0,0 +1,202 @@ +program TestSQLEscapeString; + +{$mode delphi} + +uses + SysUtils; + +var + TestCount: Integer = 0; + PassCount: Integer = 0; + FailCount: Integer = 0; + +{ --- Production code (extracted from C_Database.pas) --- } + +function SQL_Escape_String_MySQL(const sString: String): String; +begin + // Fixed: escape backslashes first, then quotes + Result := StringReplace(sString, '\', '\\', [rfReplaceAll]); + Result := StringReplace(Result, '''', '\''', [rfReplaceAll]); +end; + +function SQL_Escape_String_Standard(const sString: String): String; +begin + // SQL-standard double-quote escaping for InterBase, Firebird, Access + Result := StringReplace(sString, '''', '''''', [rfReplaceAll]); +end; + +{ --- Test helper --- } + +procedure AssertEquals(const TestName, Expected, Actual: String); +begin + Inc(TestCount); + if Expected = Actual then + begin + Inc(PassCount); + WriteLn('[PASS] ', TestName); + end + else + begin + Inc(FailCount); + WriteLn('[FAIL] ', TestName); + WriteLn(' Expected: ''', Expected, ''''); + WriteLn(' Actual: ''', Actual, ''''); + end; +end; + +{ --- MySQL tests --- } + +procedure Test_MySQL_should_escape_quote_when_input_contains_single_quote; +var + Input, Expected, Actual: String; +begin + // given + Input := 'O''Brien'; + Expected := 'O\''Brien'; + // when + Actual := SQL_Escape_String_MySQL(Input); + // then + AssertEquals('MySQL: should escape quote when input contains single quote', + Expected, Actual); +end; + +procedure Test_MySQL_should_escape_backslash_when_input_contains_backslash; +var + Input, Expected, Actual: String; +begin + // given + Input := 'C:\Users'; + Expected := 'C:\\Users'; + // when + Actual := SQL_Escape_String_MySQL(Input); + // then + AssertEquals('MySQL: should escape backslash when input contains backslash', + Expected, Actual); +end; + +procedure Test_MySQL_should_escape_both_when_input_contains_quote_and_backslash; +var + Input, Expected, Actual: String; +begin + // given + Input := 'it''s a \path'; + Expected := 'it\''s a \\path'; + // when + Actual := SQL_Escape_String_MySQL(Input); + // then + AssertEquals('MySQL: should escape both when input contains quote and backslash', + Expected, Actual); +end; + +procedure Test_MySQL_should_not_double_escape_when_input_contains_quote; +var + Input, Actual: String; + ContainsDoubleEscape: Boolean; +begin + // given: this was the original bug - quote was escaped to \' and then + // the backslash in \' was escaped again to \\', producing invalid SQL + Input := 'O''Brien'; + // when + Actual := SQL_Escape_String_MySQL(Input); + ContainsDoubleEscape := Pos('\\''', Actual) > 0; + // then + AssertEquals('MySQL: should not double-escape when input contains quote', + 'False', BoolToStr(ContainsDoubleEscape, True)); +end; + +procedure Test_MySQL_should_return_unchanged_when_no_special_chars; +var + Input, Actual: String; +begin + // given + Input := 'hello world'; + // when + Actual := SQL_Escape_String_MySQL(Input); + // then + AssertEquals('MySQL: should return unchanged when no special chars', + Input, Actual); +end; + +{ --- InterBase/Firebird/Access tests --- } + +procedure Test_Standard_should_double_quote_when_input_contains_single_quote; +var + Input, Expected, Actual: String; +begin + // given + Input := 'O''Brien'; + Expected := 'O''''Brien'; + // when + Actual := SQL_Escape_String_Standard(Input); + // then + AssertEquals('Standard: should double-quote when input contains single quote', + Expected, Actual); +end; + +procedure Test_Standard_should_not_escape_backslash_when_input_contains_backslash; +var + Input, Expected, Actual: String; +begin + // given + Input := 'C:\Users'; + Expected := 'C:\Users'; + // when + Actual := SQL_Escape_String_Standard(Input); + // then + AssertEquals('Standard: should not escape backslash when input contains backslash', + Expected, Actual); +end; + +procedure Test_Standard_should_only_escape_quotes_when_input_contains_both; +var + Input, Expected, Actual: String; +begin + // given + Input := 'it''s a \path'; + Expected := 'it''''s a \path'; + // when + Actual := SQL_Escape_String_Standard(Input); + // then + AssertEquals('Standard: should only escape quotes when input contains both', + Expected, Actual); +end; + +procedure Test_Standard_should_return_unchanged_when_no_special_chars; +var + Input, Actual: String; +begin + // given + Input := 'hello world'; + // when + Actual := SQL_Escape_String_Standard(Input); + // then + AssertEquals('Standard: should return unchanged when no special chars', + Input, Actual); +end; + +{ --- Main --- } + +begin + WriteLn('=== SQL_Escape_String Tests ==='); + WriteLn; + + WriteLn('--- MySQL ---'); + Test_MySQL_should_escape_quote_when_input_contains_single_quote; + Test_MySQL_should_escape_backslash_when_input_contains_backslash; + Test_MySQL_should_escape_both_when_input_contains_quote_and_backslash; + Test_MySQL_should_not_double_escape_when_input_contains_quote; + Test_MySQL_should_return_unchanged_when_no_special_chars; + + WriteLn; + WriteLn('--- InterBase / Firebird / Access ---'); + Test_Standard_should_double_quote_when_input_contains_single_quote; + Test_Standard_should_not_escape_backslash_when_input_contains_backslash; + Test_Standard_should_only_escape_quotes_when_input_contains_both; + Test_Standard_should_return_unchanged_when_no_special_chars; + + WriteLn; + WriteLn('=== Results: ', TestCount, ' tests, ', PassCount, ' passed, ', FailCount, ' failed ==='); + + if FailCount > 0 then + Halt(1); +end.