Repository navigation
fix(validation): read request bodies and field tables by own keys - #112
Merged
Merged
Conversation
A field named like an Object.prototype member (toString, valueOf) was always present in a request body, was validated against the inherited function, was written from it on a new row, and crashed a list sorted on it. A target default of such a name was written as a column. Fixes #65
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 controller field whose name matches an
Object.prototypemember (toString,valueOf,constructor) was found on the prototype instead of in the request body or field table. A mandatory field of that name was never reported missing. Its validator ran against the inherited function when the body omitted the field. A non-field target default of that name was written as a column. Field names come from the controller class, not the client, so a request cannot introduce such a name.Fixes #65
What changed
Every site of this family now reads own keys (
Object.hasOwnor the existingownEntryhelper from #60):src/components.tsisAbsentFromBody(Validation.MandatoryFields)!(name in body)!Object.hasOwn(body, name)src/components.tsValidation.ValidateTypes!(name in obj)!Object.hasOwn(obj, name)src/internal/properties.tswriteTargetDefaultskey in meta.fieldsownEntry(meta.fields, key)src/internal/properties.tsWritePropertiesprops and settersbodyData[key]ownEntry(bodyData, key)src/internal/list.tsisHeldByKeyedModifierfields[field]?.some(...)ownEntry(fields, field)?.some(...)The issue lists the first three. The other two came up while fixing them. With only the validation sites fixed, a new row still copied the inherited
toStringfunction from the body and answered 500. A list sorted on a sortable field namedvalueOfthrewfields[field]?.some is not a function(500).For any other field name the behaviour is unchanged. An absent field still reads as
undefined, and the edit path already filtered by own keys.flowchart LR B[request body] --> M{"Object.hasOwn(body, name)?"} M -- no --> R400[400 missing_mandatory_fields] M -- yes --> V{"validator and Object.hasOwn(body, name)?"} V -- validate --> W[WriteProperties: ownEntry body values] V -- skip --> W D[target defaults] --> F{"ownEntry(meta.fields, key)?"} F -- field --> W F -- not a field --> I[instance only, no column] W --> DB[(row)]Proof
New tests in
src/tests/components/prototype_named_field.test.tsand one added toprototype_named_filter.test.ts, run withpnpm exec ajs module test . -f dist/tests/components/<file>.test.js.Before, on
main(354796d):After:
pnpm run checks:Checks: 8 passed, 0 failed.Attention
antelopejs/no-inherited-key-lookupflags none of these sites (aninagainst a parameter typedRecord/FieldValues, and an optional-chained index into a metadata table). Widening the rule belongs in the tooling preset, not this repo.