Skip to content

fix: EXPOSED-909 Escape single quotes in locate() substring - #2946

Open
Alexandre Kohler (kwy404) wants to merge 1 commit into
JetBrains:mainfrom
kwy404:fix-locate-substring-quote
Open

Alexandre Kohler (kwy404) wants to merge 1 commit into
JetBrains:mainfrom
kwy404:fix-locate-substring-quote

Conversation

@kwy404

Copy link
Copy Markdown

Description

Summary of the change: locate() puts its substring argument inside a SQL string literal without escaping single quotes, so a substring such as ' or foo' produces invalid SQL. This PR escapes those quotes.

Detailed description:

  • Why: Every dialect except Redshift writes substring straight into a quoted SQL string literal without escaping it. For example, stringLiteral("Joe's").locate("'") on H2 renders LOCATE(''','Joe''s'), which fails with a syntax error. The same happens with POSITION (PostgreSQL), INSTR (Oracle, SQLite) and CHARINDEX (SQL Server).
  • What: The H2, MySQL, MariaDB, PostgreSQL, Oracle, SQLite and SQL Server locate implementations now double single quotes in substring, the same way RedshiftFunctionProvider.locate already does.
  • How: One line per dialect, substring becomes substring.replace("'", "''"). Added testLocateWithSingleQuote to FunctionsTests. It fails before the change on all H2 modes (H2, MySQL, PostgreSQL, MariaDB, Oracle, SQL Server) and on SQLite with a SQL syntax error, and passes after it.

Type of Change

Please mark the relevant options with an "X":

  • Bug fix
  • New feature
  • Documentation update

Updates/remove existing public API methods:

  • Is breaking change

Affected databases:

  • MariaDB
  • Mysql5
  • Mysql8
  • Oracle
  • Postgres
  • Redshift
  • SqlServer
  • H2
  • SQLite

Checklist

  • Unit tests are in place
  • The build is green (including the Detekt check)
  • All public methods affected by my PR has up to date API docs
  • Documentation for my change is up to date

Related Issues

EXPOSED-909

This branch has not been deployed

No deployments
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.

1 participant