Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/Weasel.Core/IdentifierValidation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ namespace Weasel.Core;
/// <c>'</c> closes a string literal, and object names do reach string literals on every provider --
/// the existence checks and introspection queries interpolate them (SQL Server's
/// <c>IF OBJECT_ID('...')</c>, Oracle's <c>WHERE table_name = '...'</c> inside an anonymous PL/SQL
/// block, SQLite's <c>pragma_table_info('...')</c>). Whitespace is rejected in full rather than just
/// the literal space character, so that a newline cannot introduce a <c>--</c> comment into an
/// unquoted name.
/// block, SQLite's <c>pragma_table_info('...')</c>), and PostgreSQL writes a sequence's name into
/// one when a column defaults from it (<c>DEFAULT nextval('...')</c>). Whitespace is rejected in
/// full rather than just the literal space character, so that a newline cannot introduce a
/// <c>--</c> comment into an unquoted name.
/// </para>
/// <para>
/// The rest is per-provider, because the character that closes an identifier is not: SQL Server
/// delimits with <c>[...]</c> as well as <c>"..."</c>, MySQL with backticks, Oracle and SQLite with
/// <c>"</c>. Weasel's quoting helpers do not double an embedded delimiter (SQLite's
/// delimits with <c>[...]</c> as well as <c>"..."</c>, MySQL with backticks, and Oracle, SQLite and
/// PostgreSQL with <c>"</c>. Weasel's quoting helpers do not double an embedded delimiter (SQLite's
/// <c>SchemaUtils.QuoteName</c> does, but only quotes at all for keywords, spaces, dashes and
/// leading digits), so a name carrying one does not stay inside its own quotes.
/// </para>
Expand Down
12 changes: 11 additions & 1 deletion src/Weasel.Postgresql.Tests/PostgresqlMigratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,23 @@ public void assert_identifier_length_exceeding_maximum()
/// the characters that let a name escape the statement it is written into. A '"' closes a quoted
/// identifier -- Weasel quotes without doubling an embedded quote -- and a ';' starts a new
/// statement. Both were permitted before this.
/// <para>
/// The single quote came with the move onto the shared IdentifierValidation. It is not
/// uniformity for its own sake: Table.ColumnExpression.DefaultValueFromSequence writes a
/// sequence's name into a string literal, DEFAULT nextval('{name}'), so a name carrying one
/// closes that literal. PostgreSQL's introspection queries parameterise the name, which is why
/// this is narrower here than on the providers that interpolate it.
/// </para>
/// </summary>
[Theory]
[InlineData("users\"", "a trailing double quote")]
[InlineData("\"users", "a leading double quote")]
[InlineData("us\"ers", "an embedded double quote")]
[InlineData("\"users\"", "a fully quote-wrapped name")]
[InlineData("users\"; drop table users; --", "a quote-and-semicolon payload")]
[InlineData("users'", "a trailing single quote")]
[InlineData("us'ers", "an embedded single quote")]
[InlineData("users'); drop table users; --", "a literal-breaking payload")]
[InlineData("users;", "a trailing semicolon")]
[InlineData("us;ers", "an embedded semicolon")]
public void assert_identifier_rejects_quote_and_semicolon(string name, string description)
Expand Down Expand Up @@ -153,7 +163,7 @@ public void invalid_identifier_exception_says_which_rule_was_broken()

/// <summary>
/// Names Weasel and its consumers actually generate must keep working -- the tightening is aimed at
/// two characters, not at narrowing the identifier grammar.
/// a handful of characters, not at narrowing the identifier grammar.
/// </summary>
[Theory]
[InlineData("mt_doc_user")]
Expand Down
46 changes: 15 additions & 31 deletions src/Weasel.Postgresql/PostgresqlMigrator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -334,46 +334,30 @@ public static string CreateSchemaStatementFor(string schemaName)

public override bool IsSystemColumn(string columnName) => SystemColumns.Contains(columnName);

/// <summary>
/// The character PostgreSQL delimits identifiers with. A <c>"</c> closes a quoted identifier, and
/// <see cref="SchemaUtils.QuoteName" /> does not double an embedded one -- so a name carrying one
/// does not stay inside its own quotes. The rest of what is rejected is universal and lives in
/// <see cref="IdentifierValidation" />.
/// </summary>
private const string UnsafeIdentifierCharacters = "\"";

/// <summary>
/// Validates a database object name before it is written into DDL.
/// </summary>
/// <remarks>
/// This is the only identifier check in the stack -- <see cref="DbObjectName" /> and
/// <see cref="PostgresqlObjectName" /> do no validation of their own -- so it rejects the two
/// characters that let a name escape the statement it is written into (weasel#416):
/// <list type="bullet">
/// <item>
/// <c>"</c>, which closes a quoted identifier. Weasel quotes identifiers without doubling
/// an embedded quote, so a name carrying one does not stay inside its own quotes.
/// </item>
/// <item><c>;</c>, which ends the statement and starts another.</item>
/// </list>
/// Whitespace is rejected in full rather than just the literal space it used to check, so that a
/// newline cannot introduce a <c>--</c> comment into an unquoted name either.
/// <see cref="PostgresqlObjectName" /> do no validation of their own -- so it rejects the characters
/// that let a name escape the statement it is written into: the double quote above, plus the
/// semicolon, the single quote and whitespace that <see cref="IdentifierValidation.FindProblem" />
/// rejects for every provider (weasel#416).
/// </remarks>
public override void AssertValidIdentifier(string name)
{
if (string.IsNullOrWhiteSpace(name))
{
throw new PostgresqlIdentifierInvalidException(name, "it is null, empty, or entirely whitespace");
}

foreach (var c in name)
var problem = IdentifierValidation.FindProblem(name, UnsafeIdentifierCharacters);
if (problem != null)
{
if (char.IsWhiteSpace(c))
{
throw new PostgresqlIdentifierInvalidException(name, "it contains whitespace");
}

if (c == '"')
{
throw new PostgresqlIdentifierInvalidException(name, "it contains a double quote");
}

if (c == ';')
{
throw new PostgresqlIdentifierInvalidException(name, "it contains a semicolon");
}
throw new PostgresqlIdentifierInvalidException(name, problem);
}

if (name.Length < NameDataLength)
Expand Down
Loading