Skip to content

Commit

Permalink
fix(layout): useLayoutNavigation possible perf fix
Browse files Browse the repository at this point in the history
This was caught by CodeQL:

CWE-400
CWE-730

> This regular expression that depends on library input may run slow on
> strings starting with '?' and with many repetitions of '?'.

This really shouldn't happen unless the users can configure the layout
tree themselves.
  • Loading branch information
mlaursen committed Jul 13, 2021
1 parent 75a9b0f commit 3d65e4e
Showing 1 changed file with 17 additions and 1 deletion.
18 changes: 17 additions & 1 deletion packages/layout/src/useLayoutNavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,22 @@ const noop = (): void => {
// do nothing
};

/**
* This used to just be `pathname.replace(/\?.*$/, "")` but that can apparently
* cause performance issues or a DoS attack if the pathname contains multiple
* ?`?` (shouldn't really be possible though)
*
* @remarks \@since 2.9.0
*/
const removeQueryParams = (pathname: string): string => {
const i = pathname.indexOf("?");
if (i === -1) {
return pathname;
}

return pathname.substring(0, i);
};

/**
* This is a pretty reasonable default implementation for having a navigation
* tree within the Layout component. The way it'll work is that the current
Expand Down Expand Up @@ -79,7 +95,7 @@ export function useLayoutNavigation<
pathname: string,
linkComponent: ElementType = Link
): LayoutNavigationState<T> {
const itemId = pathname.replace(/\?.*$/, "");
const itemId = removeQueryParams(pathname);
const { expandedIds, onItemExpansion, onMultiItemExpansion } =
useTreeItemExpansion(() => getParentIds(itemId, navItems));

Expand Down

1 comment on commit 3d65e4e

@vercel
Copy link

@vercel vercel bot commented on 3d65e4e Jul 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.