Skip to content

fix(game): Harmonize SQL query formatting, truncation logging, and DataPack lifecycle#1492

Open
Rushaway wants to merge 2 commits into
sbpp:mainfrom
Rushaway:game-sql
Open

fix(game): Harmonize SQL query formatting, truncation logging, and DataPack lifecycle#1492
Rushaway wants to merge 2 commits into
sbpp:mainfrom
Rushaway:game-sql

Conversation

@Rushaway
Copy link
Copy Markdown
Contributor

@Rushaway Rushaway commented Jun 1, 2026

Description

This PR updates both SourceBans++ scripting plugins to apply a consistent SQL construction and error-handling pattern.

Updated files:

  • sbpp_main.sp
  • sbpp_comms.sp

Key changes:

  • Replaced manual escaping patterns with DB.Format/SQLiteDB.Format where applicable.
  • Standardized placeholder usage:
    • %!s for internal SQL values (DatabasePrefix, table/fragment internals).
    • %s for user/runtime inputs.
  • Added/kept defensive query buffer truncation checks before DB.Query calls.
  • Harmonized truncation diagnostics to use LogError consistently across both plugins.
  • Removed early DataPack creation on code paths where query formatting can fail:
    • DataPack is now created after successful query formatting in affected command/query paths.
    • This removes unnecessary rollback-style delete usage introduced only for failed pre-query guards.

Motivation and Context

This change improves safety and consistency in SQL handling across plugins:

  • Prevents double-escaping mistakes from mixed Escape + Format patterns.
  • Enforces a clear placeholder contract for internal vs user-provided values.
  • Makes truncation failures visible in the same way across both plugins (LogError).
  • Simplifies ownership/lifecycle of DataPack objects by allocating them only when needed.

How Has This Been Tested?

  • Editor diagnostics check:
    • No errors reported for both updated files.
  • Pattern verification checks:
    • No remaining .Escape(...) usages.
    • Truncation guard logs use LogError consistently in both plugins.
  • Lifecycle verification:
    • DataPack allocation moved after successful query formatting in modified guard paths.
    • Remaining delete dataPack calls are in normal callback/cleanup flows.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

Note: If your PR touches web/** and this is your first
contribution to the web panel, the CLA bot will comment within
~30 seconds with one-line sign instructions. You only need to sign
once per repo. See CLA.md for the full text and
CONTRIBUTING.md for the rationale.

Rushaway added 2 commits June 1, 2026 08:54
… DataPack lifecycle

- replace Escape + Format patterns with DB.Format/SQLiteDB.Format in main and comms
- enforce placeholder policy: %!s for internal SQL identifiers/fragments, %s for runtime/user input
- add/keep query buffer truncation guards before query execution
- standardize truncation diagnostics on LogError across both plugins
{
// The insert was successful so delete the record from the queue
FormatEx(query, sizeof(query),
if (SQLiteDB.Format(query, sizeof(query),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be this db.Format?


char query[4096];
Format(query, sizeof(query),
if (g_hDatabase.Format(query, sizeof(query),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto, if we should use local db reference?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the two SourceBans++ SourceMod plugins to standardize SQL query construction and error handling, primarily by replacing manual escaping + Format/FormatEx patterns with Database.Format/SQLiteDB.Format and adding consistent truncation guards.

Changes:

  • Migrated query building to DB.Format / db.Format / SQLiteDB.Format with consistent placeholder usage and truncation checks.
  • Standardized truncation diagnostics to LogError and increased some query buffer sizes.
  • Adjusted DataPack allocation timing in some paths to avoid unnecessary allocation when query formatting fails.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 35 comments.

File Description
game/addons/sourcemod/scripting/sbpp_main.sp Reworked multiple ban/unban and queue queries to use DB.Format, added truncation guards, and adjusted some DataPack lifecycle decisions.
game/addons/sourcemod/scripting/sbpp_comms.sp Reworked comms queries to use Database.Format, added truncation guards, and moved DataPack allocation in unblock paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1570 to +1573
{
LogError("Query_UnBlockSelect update query truncated");
continue;
}
Comment on lines +1300 to +1303
{
LogError("SelectBanIpCallback insert query truncated");
return;
}
Comment on lines +1309 to +1312
{
LogError("SelectBanIpCallback insert query truncated");
return;
}
Comment on lines +1517 to +1520
{
LogError("SelectAddbanCallback insert query truncated");
return;
}
Comment on lines +1526 to +1529
{
LogError("SelectAddbanCallback insert query truncated");
return;
}
Comment on lines +1309 to +1312
{
LogError("SelectBanIpCallback insert query truncated");
return;
}
Comment on lines +1517 to +1520
{
LogError("SelectAddbanCallback insert query truncated");
return;
}
Comment on lines +1526 to +1529
{
LogError("SelectAddbanCallback insert query truncated");
return;
}
Comment on lines +2619 to +2622
{
LogError("UTIL_InsertBan query truncated");
return;
}
Comment on lines +2628 to +2631
{
LogError("UTIL_InsertBan query truncated");
return;
}
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.

3 participants