feat(javascript): add configurable size guardrails#3426
feat(javascript): add configurable size guardrails#3426shiavm006 wants to merge 9 commits intoapache:mainfrom
Conversation
shiavm006
changed the title
|
@chaokunyang just check it once |
|
@shiavm006 Please make all ci pass before request the code review |
|
and there is not tests, which also need be addressed |
ya i am sorry for that willmake sure of that from next time |
|
i think now u can ci check i have fixed them there was some syntax error that was some issue in logic fixed that |
|
@chaokunyang @theweipeng could u check this out once |
| const maxEntriesVar = this.scope.uniqueName("maxMapEntries"); | ||
| return ` | ||
| let ${count} = ${this.builder.reader.readVarUint32Small7()}; | ||
| const ${maxEntriesVar} = ${foryName}.config.maxMapEntries; |
There was a problem hiding this comment.
You can generate fixed code.
`
if (${count} > ${this.fory.config.maxMapEntries}) {
}
the code generated will be like:
if (count > 10) {
}
`
There was a problem hiding this comment.
Try to calculate the constants well during the code generation stage to reduce the consumption in runtime
| const flags = this.scope.uniqueName("flags"); | ||
| const idx = this.scope.uniqueName("idx"); | ||
| const refFlag = this.scope.uniqueName("refFlag"); | ||
| const maxLenConfig = this.builder.fory.config.maxCollectionLength; |
There was a problem hiding this comment.
We use maxCollectionSize/maxBinarySize now. could you rename it?
There was a problem hiding this comment.
renamed it now ig its good-to merge
javascript/packages/fory/lib/fory.ts
Outdated
| useSliceString: Boolean(config?.useSliceString), | ||
| hooks: config?.hooks || {}, | ||
| compatible: Boolean(config?.compatible), | ||
| maxStringBytes: config?.maxStringBytes, |
There was a problem hiding this comment.
Delete this option, use maxBinarySize for string preallocation check
3 participants
Summary
Configfields for string byte length, collection length, and map entry count.Related issues #3414