mirror of
https://github.com/pezkuwichain/pezkuwi-api.git
synced 2026-10-20 18:56:29 +00:00
Remove the CodeQL findings in typegen and PortableRegistry
- typegen formatType: the inline-struct shortcut test /^{.+:.+}/ has two
unbounded wildcards and backtracks quadratically on input such as
'{{a:a:a:...' (370 ms at 20k repetitions). isInlineStruct checks the
same thing with indexOf: an opening brace, a colon at index 2 or later
and a closing brace at least two characters after it, all on the
first line. 500k random inputs give the same answer as the regex; a
test keeps it under 100 ms at 100k repetitions and goes red on the
regex.
- PortableRegistry: the type definition cache was a plain object
written as typeDefs[lookupIndex][key]. lookupIndex is always a number,
but nothing in the shape said so; it is now a Map, and the definition
is filled through its own reference.
This commit is contained in:
1 parent
55702e7638
commit
3ff2be4b71
3 files changed
+79
-37
No files matched your search
@@ -5,7 +5,7 @@
|
||||
|
||||
import { TypeRegistry } from '@pezkuwi/types';
|
||||
|
||||
import { formatType } from './formatting.js';
|
||||
import { formatType, isInlineStruct } from './formatting.js';
|
||||
|
||||
describe('formatType', (): void => {
|
||||
const registry = new TypeRegistry();
|
||||
@@ -28,3 +28,29 @@ describe('formatType', (): void => {
|
||||
).toEqual('ITuple<[AccountId, ITuple<[Balance, u32]>, u64]>');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isInlineStruct', (): void => {
|
||||
it('matches what /^{.+:.+}/ matched', (): void => {
|
||||
for (const [type, expected] of [
|
||||
['{ a: u32 }', true],
|
||||
['{"a":"u32"}', true],
|
||||
['{a:}', false],
|
||||
['{:a}', false],
|
||||
['{a:b', false],
|
||||
['{a:b}c', true],
|
||||
['x{a:b}', false],
|
||||
['{a\n:b}', false],
|
||||
['{a:b\n}', false],
|
||||
['(a: b)', false]
|
||||
] as const) {
|
||||
expect([type, isInlineStruct(type)]).toEqual([type, expected]);
|
||||
}
|
||||
});
|
||||
|
||||
it('is linear on input that made the regex backtrack', (): void => {
|
||||
const start = Date.now();
|
||||
|
||||
expect(isInlineStruct(`{{${'a:'.repeat(100_000)}`)).toBe(false);
|
||||
expect(Date.now() - start < 100).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -301,6 +301,19 @@ const formatters: Record<TypeDefInfo, (registry: Registry, typeDef: TypeDef, def
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* `{ name: Type }` written inline: an opening brace, a colon and then a closing
|
||||
* brace on the first line. This is what /^{.+:.+}/ matched, without the
|
||||
* quadratic backtracking that regex has on input like '{{a:a:a:...'.
|
||||
*/
|
||||
/** @internal */
|
||||
export function isInlineStruct (type: string): boolean {
|
||||
const line = type.split(/[\n\r\u2028\u2029]/, 1)[0];
|
||||
const colon = line.indexOf(':', 2);
|
||||
|
||||
return line.startsWith('{') && colon !== -1 && line.includes('}', colon + 2);
|
||||
}
|
||||
|
||||
/**
|
||||
* Correctly format a given type
|
||||
*/
|
||||
@@ -314,7 +327,7 @@ export function formatType (registry: Registry, definitions: Record<string, Modu
|
||||
|
||||
// If type is "unorthodox" (i.e. `{ something: any }` for an Enum input or `[a | b | c, d | e | f]` for a Tuple's similar types),
|
||||
// we return it as-is
|
||||
if (withShortcut && /(^{.+:.+})|^\([^,]+\)|^\(.+\)\[\]|^\[.+\]/.exec(_type) && !/\[\w+;\w+\]/.exec(_type)) {
|
||||
if (withShortcut && (isInlineStruct(_type) || /^\([^,]+\)|^\(.+\)\[\]|^\[.+\]/.exec(_type)) && !/\[\w+;\w+\]/.exec(_type)) {
|
||||
return rebrandTypeName(_type);
|
||||
}
|
||||
|
||||
|
||||
@@ -537,7 +537,7 @@ export class PortableRegistry extends Struct implements ILookup {
|
||||
#lookups: Record<string, LookupString>;
|
||||
#names: Record<number, string>;
|
||||
#params: TypeInfoParams;
|
||||
#typeDefs: Record<number, TypeDef> = {};
|
||||
readonly #typeDefs = new Map<number, TypeDef>();
|
||||
#types: Record<number, PortableType>;
|
||||
|
||||
constructor (registry: Registry, value?: Uint8Array, isContract?: boolean) {
|
||||
@@ -613,43 +613,46 @@ export class PortableRegistry extends Struct implements ILookup {
|
||||
*/
|
||||
public getTypeDef (lookupId: SiLookupTypeId | LookupString | number): TypeDef {
|
||||
const lookupIndex = this.#getLookupId(lookupId);
|
||||
const cached = this.#typeDefs.get(lookupIndex);
|
||||
|
||||
if (!this.#typeDefs[lookupIndex]) {
|
||||
const lookupName = this.#names[lookupIndex];
|
||||
const empty = {
|
||||
info: TypeDefInfo.DoNotConstruct,
|
||||
lookupIndex,
|
||||
lookupName,
|
||||
type: this.registry.createLookupType(lookupIndex)
|
||||
};
|
||||
|
||||
// Set named items since we will get into circular lookups along the way
|
||||
if (lookupName) {
|
||||
this.#typeDefs[lookupIndex] = empty;
|
||||
}
|
||||
|
||||
const extracted = this.#extract(this.getSiType(lookupId), lookupIndex);
|
||||
|
||||
// For non-named items, we only set this right at the end
|
||||
if (!lookupName) {
|
||||
this.#typeDefs[lookupIndex] = empty;
|
||||
}
|
||||
|
||||
Object.keys(extracted).forEach((k): void => {
|
||||
if (k !== 'lookupName' || extracted[k]) {
|
||||
// these are safe since we are looking through the keys as set
|
||||
this.#typeDefs[lookupIndex][k as 'info'] = extracted[k as 'info'];
|
||||
}
|
||||
});
|
||||
|
||||
// don't set lookupName on lower-level, we want to always direct to the type
|
||||
if (extracted.info === TypeDefInfo.Plain) {
|
||||
this.#typeDefs[lookupIndex].lookupNameRoot = this.#typeDefs[lookupIndex].lookupName;
|
||||
delete this.#typeDefs[lookupIndex].lookupName;
|
||||
}
|
||||
if (cached) {
|
||||
return cached;
|
||||
}
|
||||
|
||||
return this.#typeDefs[lookupIndex];
|
||||
const lookupName = this.#names[lookupIndex];
|
||||
const typeDef: TypeDef = {
|
||||
info: TypeDefInfo.DoNotConstruct,
|
||||
lookupIndex,
|
||||
lookupName,
|
||||
type: this.registry.createLookupType(lookupIndex)
|
||||
};
|
||||
|
||||
// Set named items since we will get into circular lookups along the way
|
||||
if (lookupName) {
|
||||
this.#typeDefs.set(lookupIndex, typeDef);
|
||||
}
|
||||
|
||||
const extracted = this.#extract(this.getSiType(lookupId), lookupIndex);
|
||||
|
||||
// For non-named items, we only set this right at the end
|
||||
if (!lookupName) {
|
||||
this.#typeDefs.set(lookupIndex, typeDef);
|
||||
}
|
||||
|
||||
Object.keys(extracted).forEach((k): void => {
|
||||
if (k !== 'lookupName' || extracted[k]) {
|
||||
// these are safe since we are looking through the keys as set
|
||||
typeDef[k as 'info'] = extracted[k as 'info'];
|
||||
}
|
||||
});
|
||||
|
||||
// don't set lookupName on lower-level, we want to always direct to the type
|
||||
if (extracted.info === TypeDefInfo.Plain) {
|
||||
typeDef.lookupNameRoot = typeDef.lookupName;
|
||||
delete typeDef.lookupName;
|
||||
}
|
||||
|
||||
return typeDef;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in new issue
Block a user