Skip to content

Commit

Permalink
fix: safely handle nested removal of menu routes (#19885)
Browse files Browse the repository at this point in the history
MenuRegistry.filterClientViews removes inaccessible routes 
and their child routes. When iterating the list of all routes,
 a NPE can occur if a child route has already been 
removed together with its parent route. 
Add a check that prevents NPE for removed children.
  • Loading branch information
sissbruecker committed Sep 4, 2024
1 parent b00b97c commit c120a07
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,10 @@ private static void filterClientViews(

Set<String> clientEntries = new HashSet<>(configurations.keySet());
for (String key : clientEntries) {
if (!configurations.containsKey(key)) {
// view may have been removed together with parent
continue;
}
AvailableViewInfo viewInfo = configurations.get(key);
boolean routeValid = validateViewAccessible(viewInfo,
isUserAuthenticated, vaadinRequest::isUserInRole);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,19 @@ public void getMenuItemsContainsExpectedClientPaths() throws IOException {
assertClientRoutes(menuItems);
}

@Test
public void getMenuItemsWithNestedFiltering_doesNotThrow()
throws IOException {
File generated = tmpDir.newFolder(GENERATED);
File clientFiles = new File(generated, FILE_ROUTES_JSON_NAME);
Files.writeString(clientFiles.toPath(), nestedLoginRequiredRouteFile);

Map<String, AvailableViewInfo> menuItems = new MenuRegistry()
.getMenuItems(true);

Assert.assertEquals(0, menuItems.size());
}

@Test
public void getMenuItemsNoFilteringContainsAllClientPaths()
throws IOException {
Expand Down Expand Up @@ -462,4 +475,44 @@ public Instantiator getInstantiator() {
}
]
""";

String nestedLoginRequiredRouteFile = """
[
{
"route": "",
"params": {},
"title": "current Title",
"children": [
{
"route": "",
"loginRequired": true,
"params": {},
"title": "navigate"
},
{
"route": "admin",
"loginRequired": true,
"title": "Admin",
"params": {},
"children": [
{
"route": "planets",
"loginRequired": true,
"title": "Planets",
"params": {},
"children": [
{
"route": "",
"loginRequired": true,
"title": "Planets",
"params": {}
}
]
}
]
}
]
}
]
""";
}

0 comments on commit c120a07

Please sign in to comment.