Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 3 additions & 3 deletions TShockAPI/DB/BanManager.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*
/*
TShock, a server mod for Terraria
Copyright (C) 2011-2019 Pryaxis & TShock Contributors

Expand Down Expand Up @@ -106,7 +106,7 @@ public void TryConvertBans()
{
SqlType.Mysql => database.QueryScalar<int>("SELECT COUNT(table_name) FROM information_schema.tables WHERE table_schema = @0 and table_name = 'Bans'", TShock.Config.Settings.MySqlDbName),
SqlType.Sqlite => database.QueryScalar<int>("SELECT COUNT(name) FROM sqlite_master WHERE type='table' AND name = 'Bans'"),
SqlType.Postgres => database.QueryScalar<int>("SELECT COUNT(table_name) FROM information_schema.tables WHERE table_name = 'Bans'"),
SqlType.Postgres => database.QueryScalar<int>("SELECT COUNT(table_name) FROM information_schema.tables WHERE table_schema=current_schema() AND table_name ILIKE 'Bans'"),
};

if (res != 0)
Expand Down Expand Up @@ -298,7 +298,7 @@ public AddBanResult InsertBan(BanPreAddEventArgs args)
{
SqlType.Mysql => /*lang=mysql*/"; SELECT LAST_INSERT_ID();",
SqlType.Sqlite => /*lang=sqlite*/"; SELECT last_insert_rowid();",
SqlType.Postgres => /*lang=postgresql*/"RETURNING \"Identifier\";",
SqlType.Postgres => /*lang=postgresql*/" RETURNING TicketNumber;",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The RETURNING clause should quote TicketNumber consistently with how PostgresQueryBuilder.EscapeColumnName stores the column (lowercased, double-quoted). While PostgreSQL folds unquoted TicketNumber to ticketnumber at runtime and the match succeeds today, using the same quoting convention eliminates the silent dependency on case-folding.

Suggested change
SqlType.Postgres => /*lang=postgresql*/" RETURNING TicketNumber;",
SqlType.Postgres => /*lang=postgresql*/" RETURNING \"ticketnumber\";",

_ => null
};

Expand Down
15 changes: 11 additions & 4 deletions TShockAPI/DB/Queries/GenericQueryBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*
/*
TShock, a server mod for Terraria
Copyright (C) 2011-2025 Pryaxis & TShock Contributors

Expand Down Expand Up @@ -39,6 +39,13 @@ public abstract class GenericQueryBuilder : IQueryBuilder
/// <returns></returns>
protected abstract string EscapeTableName(string table);

/// <summary>
/// Escapes a column identifier for the current SQL dialect.
/// </summary>
/// <param name="column">The column name to escape.</param>
/// <returns>The escaped column identifier.</returns>
protected virtual string EscapeColumnName(string column) => column;

/// <inheritdoc />
public abstract string CreateTable(SqlTable table);

Expand All @@ -55,7 +62,7 @@ public string AlterTable(SqlTable from, SqlTable to)
var create = CreateTable(to);
// combine all columns in the 'from' variable excluding ones that aren't in the 'to' variable.
// exclude the ones that aren't in 'to' variable because if the column is deleted, why try to import the data?
var columns = string.Join(", ", from.Columns.Where(c => to.Columns.Any(c2 => c2.Name == c.Name)).Select(c => $"`{c.Name}`"));
var columns = string.Join(", ", from.Columns.Where(c => to.Columns.Any(c2 => c2.Name.Equals(c.Name, StringComparison.OrdinalIgnoreCase))).Select(c => EscapeColumnName(c.Name)));
var insert = "INSERT INTO {0} ({1}) SELECT {1} FROM {2}".SFormat(escapedTable, columns, tmpTable);
var drop = "DROP TABLE {0}".SFormat(tmpTable);
return "{0}; {1}; {2}; {3};".SFormat(alter, create, insert, drop);
Expand Down Expand Up @@ -131,6 +138,6 @@ public string InsertValues(string table, List<SqlValue> values)
/// <param name="wheres"></param>
/// <returns></returns>
protected static string BuildWhere(List<SqlValue> wheres) => wheres.Count > 0
? string.Empty
: "WHERE {0}".SFormat(string.Join(", ", wheres.Select(v => $"{v.Name} = {v.Value}")));
? "WHERE {0}".SFormat(string.Join(" AND ", wheres.Select(v => $"{v.Name} = {v.Value}")))
: string.Empty;
}
5 changes: 4 additions & 1 deletion TShockAPI/DB/Queries/MysqlQueryBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*
/*
TShock, a server mod for Terraria
Copyright (C) 2011-2025 Pryaxis & TShock Contributors

Expand Down Expand Up @@ -83,6 +83,9 @@ public override string DbTypeToString(MySqlDbType type, int? length)
throw new NotImplementedException(Enum.GetName(typeof(MySqlDbType), type));
}

/// <inheritdoc />
protected override string EscapeColumnName(string column) => $"`{column}`";

/// <inheritdoc />
protected override string EscapeTableName(string table) => table.SFormat("`{0}`", table);
}
9 changes: 6 additions & 3 deletions TShockAPI/DB/Queries/PostgresQueryBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*
/*
TShock, a server mod for Terraria
Copyright (C) 2011-2025 Pryaxis & TShock Contributors

Expand Down Expand Up @@ -46,6 +46,9 @@ public class PostgresQueryBuilder : GenericQueryBuilder
_ => throw new NotImplementedException(Enum.GetName(typeof(MySqlDbType), type))
};

/// <inheritdoc />
protected override string EscapeColumnName(string column) => $"\"{column.ToLowerInvariant().Replace("\"", "\"\"")}\"";

/// <inheritdoc />
protected override string EscapeTableName(string table) => table.SFormat("\"{0}\"", table);

Expand All @@ -68,15 +71,15 @@ public override string CreateTable(SqlTable table)
dataType = DbTypeToString(c.Type, c.Length);
}

return "{0} {1} {2} {3} {4}".SFormat(c.Name,
return "{0} {1} {2} {3} {4}".SFormat(EscapeColumnName(c.Name),
dataType,
c.Primary ? "PRIMARY KEY" : "",
c.NotNull && !c.AutoIncrement ? "NOT NULL" : "", // SERIAL implies NOT NULL
c.DefaultCurrentTimestamp ? "DEFAULT CURRENT_TIMESTAMP" : "");
});

string[] uniques = table.Columns
.Where(c => c.Unique).Select(c => c.Name)
.Where(c => c.Unique).Select(c => EscapeColumnName(c.Name))
.ToArray(); // No re-enumeration

return $"CREATE TABLE {EscapeTableName(table.Name)} ({string.Join(", ", columns)} {(uniques.Any() ? ", UNIQUE({0})".SFormat(string.Join(", ", uniques)) : "")})";
Expand Down
5 changes: 4 additions & 1 deletion TShockAPI/DB/Queries/SqliteQueryBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*
/*
TShock, a server mod for Terraria
Copyright (C) 2011-2025 Pryaxis & TShock Contributors

Expand Down Expand Up @@ -94,6 +94,9 @@ public override string DbTypeToString(MySqlDbType type, int? length)
throw new NotImplementedException(Enum.GetName(typeof(MySqlDbType), type));
}

/// <inheritdoc />
protected override string EscapeColumnName(string column) => $"\'{column}\'";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Single-quoted column names corrupt data during schema migration

In SQL, single quotes denote string literals, not identifiers. When AlterTable generates its copy query it reuses the column list in both positions: INSERT INTO 'tablename' ('col1', 'col2') SELECT 'col1', 'col2' FROM 'tmpTableName'. SQLite resolves 'col1' in the SELECT expression as a string constant, so every row in the migrated table ends up with the column name as its value instead of the actual data. The identifier-escaping character for SQLite is " (double-quote, per SQL standard and the SQLite docs) or backtick — single quotes are not valid here.

Suggested change
protected override string EscapeColumnName(string column) => $"\'{column}\'";
/// <inheritdoc />
protected override string EscapeColumnName(string column) => $"\"{column}\"";


/// <summary>
/// Escapes the table name
/// </summary>
Expand Down
15 changes: 11 additions & 4 deletions TShockAPI/DB/RegionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ public class RegionManager

private IDbConnection database;

private string GroupsColumnName => database.GetSqlType() switch
{
SqlType.Mysql => "`Groups`",
SqlType.Postgres => "\"groups\"",
_ => "Groups"
};

internal RegionManager(IDbConnection db)
{
database = db;
Expand Down Expand Up @@ -136,7 +143,7 @@ public bool AddRegion(int tx, int ty, int width, int height, string regionname,
try
{
database.Query(
"INSERT INTO Regions (X1, Y1, width, height, RegionName, WorldID, UserIds, Protected, `Groups`, Owner, Z) VALUES (@0, @1, @2, @3, @4, @5, @6, @7, @8, @9, @10);",
$"INSERT INTO Regions (X1, Y1, width, height, RegionName, WorldID, UserIds, Protected, {GroupsColumnName}, Owner, Z) VALUES (@0, @1, @2, @3, @4, @5, @6, @7, @8, @9, @10);",
tx, ty, width, height, regionname, worldid, "", 1, "", owner, z);
int id;
using (QueryResult res = database.QueryReader("SELECT Id FROM Regions WHERE RegionName = @0 AND WorldID = @1", regionname, worldid))
Expand Down Expand Up @@ -583,7 +590,7 @@ public bool AllowGroup(string regionName, string groupName)
{
string mergedGroups = "";
using (
var reader = database.QueryReader("SELECT `Groups` FROM Regions WHERE RegionName=@0 AND WorldID=@1", regionName,
var reader = database.QueryReader($"SELECT {GroupsColumnName} FROM Regions WHERE RegionName=@0 AND WorldID=@1", regionName,
Main.worldID.ToString()))
{
if (reader.Read())
Expand All @@ -599,7 +606,7 @@ public bool AllowGroup(string regionName, string groupName)
mergedGroups += ",";
mergedGroups += groupName;

int q = database.Query("UPDATE Regions SET `Groups`=@0 WHERE RegionName=@1 AND WorldID=@2", mergedGroups,
int q = database.Query($"UPDATE Regions SET {GroupsColumnName}=@0 WHERE RegionName=@1 AND WorldID=@2", mergedGroups,
regionName, Main.worldID.ToString());

Region r = GetRegionByName(regionName);
Expand Down Expand Up @@ -628,7 +635,7 @@ public bool RemoveGroup(string regionName, string group)
{
r.RemoveGroup(group);
string groups = string.Join(",", r.AllowedGroups);
int q = database.Query("UPDATE Regions SET `Groups`=@0 WHERE RegionName=@1 AND WorldID=@2", groups,
int q = database.Query($"UPDATE Regions SET {GroupsColumnName}=@0 WHERE RegionName=@1 AND WorldID=@2", groups,
regionName, Main.worldID.ToString());
if (q > 0)
return true;
Expand Down