Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pipe.parser): RangeError: Maximum call stack size exceeded on nested templates #34

Merged
merged 1 commit into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 60 additions & 30 deletions src/parsers/pipe.parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ import {
LiteralArray,
Interpolation,
Call,
TmplAstIfBlockBranch,
TmplAstSwitchBlockCase
TmplAstIfBlock,
TmplAstSwitchBlock,
TmplAstDeferredBlock,
TmplAstForLoopBlock,
TmplAstElement
} from '@angular/compiler';

import { ParserInterface } from './parser.interface.js';
Expand All @@ -21,6 +24,58 @@ import { isPathAngularComponent, extractComponentInlineTemplate } from '../utils

export const TRANSLATE_PIPE_NAMES = ['translate', 'marker'];

function traverseAstNodes<RESULT extends unknown, NODE extends TmplAstNode | TmplAstElement>(
nodes: (NODE | null)[],
visitor: (node: NODE) => RESULT[],
accumulator: RESULT[] = []
): RESULT[] {
for (const node of nodes) {
if (node) {
traverseAstNode(node, visitor, accumulator);
}
}

return accumulator;
}

function traverseAstNode<RESULT extends unknown, NODE extends TmplAstNode | TmplAstElement>(
node: NODE,
visitor: (node: NODE) => RESULT[],
accumulator: RESULT[] = []
): RESULT[] {
accumulator.push(...visitor(node));

const children: TmplAstNode[] = [];
// children of templates, html elements or blocks
if ('children' in node && node.children) {
children.push(...node.children);
}

// contents of @for extra sibling block @empty
if (node instanceof TmplAstForLoopBlock) {
children.push(node.empty);
}

// contents of @defer extra sibling blocks @error, @placeholder and @loading
if (node instanceof TmplAstDeferredBlock) {
children.push(node.error);
children.push(node.loading);
children.push(node.placeholder);
}

// contents of @if and @else (ignoring the @if(...) condition statement though)
if (node instanceof TmplAstIfBlock) {
children.push(...node.branches.flatMap((inner) => inner.children));
}

// contents of @case blocks (ignoring the @switch(...) statement though)
if (node instanceof TmplAstSwitchBlock) {
children.push(...node.cases.flatMap((inner) => inner.children));
}

return traverseAstNodes(children, visitor, accumulator);
}

export class PipeParser implements ParserInterface {
public extract(source: string, filePath: string): TranslationCollection {
if (filePath && isPathAngularComponent(filePath)) {
Expand All @@ -29,7 +84,9 @@ export class PipeParser implements ParserInterface {

let collection: TranslationCollection = new TranslationCollection();
const nodes: TmplAstNode[] = this.parseTemplate(source, filePath);
const pipes: BindingPipe[] = nodes.map((node) => this.findPipesInNode(node)).flat();

const pipes = traverseAstNodes(nodes, (node) => this.findPipesInNode(node));

pipes.forEach((pipe) => {
this.parseTranslationKeysFromPipe(pipe).forEach((key: string) => {
collection = collection.add(key, '', filePath);
Expand All @@ -41,29 +98,6 @@ export class PipeParser implements ParserInterface {
protected findPipesInNode(node: any): BindingPipe[] {
const ret: BindingPipe[] = [];

const nodeChildren = node?.children ?? [];

// @if and @switch blocks
const nodeBranchesOrCases: TmplAstIfBlockBranch[] | TmplAstSwitchBlockCase[] = node?.branches ?? node?.cases ?? [];

// @for blocks
const emptyBlockChildren = node?.empty?.children ?? [];

// @deferred blocks
const errorBlockChildren = node?.error?.children ?? [];
const loadingBlockChildren = node?.loading?.children ?? [];
const placeholderBlockChildren = node?.placeholder?.children ?? [];

nodeChildren.push(...emptyBlockChildren, ...errorBlockChildren, ...loadingBlockChildren, ...placeholderBlockChildren);

if (nodeChildren.length > 0) {
ret.push(...this.extractPipesFromChildNodes(nodeChildren));
}

nodeBranchesOrCases.forEach((branch) => {
ret.push(...this.extractPipesFromChildNodes(branch.children));
});

if (node?.value?.ast) {
ret.push(...this.getTranslatablesFromAst(node.value.ast));
}
Expand Down Expand Up @@ -93,10 +127,6 @@ export class PipeParser implements ParserInterface {
return ret;
}

protected extractPipesFromChildNodes(nodeChildren: TmplAstNode[]) {
return nodeChildren.map((childNode) => this.findPipesInNode(childNode)).flat();
}

protected parseTranslationKeysFromPipe(pipeContent: BindingPipe | LiteralPrimitive | Conditional): string[] {
const ret: string[] = [];
if (pipeContent instanceof LiteralPrimitive) {
Expand Down
13 changes: 13 additions & 0 deletions tests/parsers/pipe.parser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,5 +369,18 @@ describe('PipeParser', () => {
'else.block'
]);
});

it('should handle ast with arbitrary depth without hitting the call stack limit', () => {
const depth = 500;
const contents = `
${Array(depth).fill('<i>').join('')}
{{ 'deep' | translate }}
${Array(depth).fill('</i>').join('')}
`;

const keys = parser.extract(contents, templateFilename)?.keys();
expect(contents).to.contain('<i><i><i><i><i><i>');
expect(keys).to.deep.equal(['deep']);
});
});
});