BrighterCommand / BrighterCommand/Brighter
MsSqlQueueBuilder.GetExistsQuery formats both names into SQL string literals
- Dominant language
- C#
- Stars
- 2.5k
- Forks
- 296
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 21
Description
`src/Paramore.Brighter.MessagingGateway.MsSql/MsSqlQueueBuilder.cs:84`:
```csharp
public static string GetExistsQuery(string queueTableName, string schemaName = "dbo") =>
string.Format(QUEUE_EXISTS_SQL, queueTableName, schemaName);
```
and the template it formats into:
```sql
WHERE t.name = '{0}' AND s.name = '{1}'
```
Both names land inside SQL string literals with no escaping, so a name containing a single quote changes the statement. `GetDDL` and `GetIndexDDL` have the same shape into `CREATE TABLE [{0}]`, where the character that matters is `]` rather than `'`.
### Why this is worth fixing rather than guarding at each call site
`MsSqlQueueBuilder` is **public API** — deliberately so, because the MSSQL gateway had no provisioning path and callers were expected to run the DDL themselves. So the guard cannot live in a caller without every caller re-deriving it, and each one getting it slightly differently.
A worked example of getting it wrong: while moving the sample's provisioner into the gateway (#4343) I ported a guard of `^[A-Za-z_][A-Za-z0-9_]{0,127}$`, which looks careful and is wrong — a **bracketed** identifier legally holds hyphens, and this repository's own test suite names queue tables `queue_test_`. The existing suite caught it. That is the argument for one guard, in the builder, next to the `string.Format`.
### Suggested fix
- `GetExistsQuery`: bind the two names as parameters rather than formatting them, or at minimum escape `'` as `''`.
- `GetDDL` / `GetIndexDDL`: escape `]` as `]]`, which is SQL Server's own convention for a bracketed identifier, and reject a name over 128 characters.
A parameterised existence check also fixes a second, quieter problem: `GetExistsQuery` defaults `schemaName` to a literal `dbo`, while every statement the gateway issues against the queue is **unqualified** and so resolves through the caller's default schema. A login whose default schema is not `dbo` gets an existence check that disagrees with the runtime. #4343 sidesteps this by asking `SCHEMA_NAME()`, but `GetExistsQuery` is public and still carries the `dbo` default.
Raised in review of #4331; #4343 is the first production caller of this family of methods.
Contributor guide
Research direction
Start in src/Paramore.Brighter.MessagingGateway.MsSql/MsSqlQueueBuilder.cs, focusing on GetExistsQuery, GetDDL, and GetIndexDDL, then inspect the existing tests and the production caller from #4343. Verify behavior for quoted names, bracketed identifiers, names over 128 characters, and schemas other than dbo; done means generated SQL remains valid and existence checks match the gateway's schema resolution.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100