Repository navigation
fix(server): reject a NUL or a lone surrogate in a text value with a 422 - #122
Merged
Merged
Conversation
The domain checks only the length of a text value. Postgres text cannot hold U+0000, and UTF-8 cannot encode a lone surrogate, so the save fails and the API gives 500. These tests fail until the domain refuses both: - StringColumnLengthTests writes U+0000, U+D800, and U+DC00 to each caller column and expects a ValidationException on the request field. - CharacterPostgresCannotStoreTests sends a NUL in each text field of each write endpoint over HTTP and expects 422 on the field. - The gRPC AddTenant and MCP update_tenant tests expect an invalid argument on the name.
Postgres text cannot hold U+0000, and UTF-8 cannot encode a lone surrogate. The domain checked only the length of a text value, so such a value got to Postgres and the API gave 500. ValidationException.ThrowWhenHasCharacterPostgresCannotStore refuses both. Each text value that a request sets calls it: the first name, last name, identifier, and username of a user, the email, the tenant name, and the agent name and client ID. Other control characters stay allowed, because Postgres stores them. AddTenant, AddUser, and InitiateUserAndTenant ran their unique-value queries with the raw request value before the entity validated it, so the query failed first. They now create the entity first and query with its value. UpdateTenant now also queries with the value of the entity. AddUser and AddAgent sent the requested tenant ID to a query before any check. A caller that manages all tenants could send a NUL there. An ID that is not a valid tenant ID now gets "The tenant does not exist." without a query.
Owner
Author
|
Review gate: two independent review passes found no high or medium findings. The first pass found three low ones (the tenant ID query in |
…cters that stay allowed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A text value with U+0000 or a lone UTF-16 surrogate passed the domain checks, then Postgres refused it, and the API gave 500 in place of 422. Postgres text cannot hold U+0000, and UTF-8 cannot encode a lone surrogate. The domain checked only the length. Also,
AddTenant,AddUser, andInitiateUserAndTenantsent the raw request value to a unique-value query before the entity validated it, so that query failed first.Closes #104
What changed
ValidationException.ThrowWhenHasCharacterPostgresCannotStoreis the one shared check. Each text value that a request sets calls it, next to its length check, inUser,Tenant,Agent, andEmailAddress. The message is "A first name has a character that is not allowed.", with the name of the field.AddTenant,AddUser, andInitiateUserAndTenantcreate the entity first and query with its value, asUpdateTenantandAddAgentalready did.docs/backend/domain-entities.mdand the validation rules indocs/backend/features-and-cqrs.mdstate the check and the order.Character rule
The domain refuses U+0000 and each unpaired surrogate (U+D800 to U+DFFF without its pair). These are the only characters that Postgres cannot store. A valid surrogate pair, such as an emoji, is allowed.
Other control characters stay allowed. Postgres stores U+0001 to U+001F, U+007F, and U+0085, and the repro gave 201 for each of them in a first name. None of them gives a 500. Whether a name can hold a tab or a line feed is a product rule for each field, and a later multi-line field needs them. This PR does not make that rule.
Scope
tenantIdthat is not a valid tenant ID before the query inAddUserandAddAgent, with the same "The tenant does not exist." message.entityIdof the audit page still gives 500. See fix(server): a NUL in a filter, an ID, or the audit entity ID gives a 500 in place of a 400 or a 404 #121.Blast Radius
Only values that failed with 500 now fail with 422. In the three reordered handlers, a request with a taken value and a field that is not valid now gets 422, not 409.
Verification
"firstName":"a\u0000b"onPOST /api/v1/users(22P05: unsupported Unicode escape sequence) and for a tenant name onPUT /api/v1/tenants/{id}(22021: invalid byte sequence for encoding "UTF8": 0x00). gRPCAddTenantgaveUnknown, and MCPupdate_tenantgave "An error occurred invoking 'update_tenant'."errors.firstNameanderrors.name, gRPC givesInvalidArgumentwith a field violation onname, and MCP givesinvalid_argument.StringColumnLengthTestssends U+0000, U+D800, and U+DC00 to each of the 8 caller columns (24 failed before the fix, 0 after).CharacterPostgresCannotStoreTestssends a NUL to each of the 19 text fields of the 6 write endpoints (20 of 20 failed before with 500, all pass after), plus the twotenantIdcases (500 before, 422 after).dotnet test --solution FullstackTemplate.slnx: 1767 passed.