demus/openhab-core
1
0
Fork 0
mirror of https://github.com/danieldemus/openhab-core.git synced 2025-01-10 13:21:53 +01:00
Code Issues 1 Packages Projects Releases Wiki Activity

Fix StackOverflowError in SemanticsMetadataProvider (#3748)

Browse Source 
In case of cyclic/recursive membership of groups (i.e. a group has itself or a child as parent) the recursive processing of the items leads to an `StackOverflowError` (reported in the forum and the german Facebook group). Also this is clearly a configuration error it should not result in the bundle not starting.

Signed-off-by: Jan N. Klug <github@klug.nrw>
This commit is contained in:
J-N-K 2023-08-12 11:49:12 +02:00 committed by GitHub
parent 04105c1b1a
commit caf13da88a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 74 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
bundles/org.openhab.core.semantics/pom.xml
View File

@ -20,6 +20,12 @@
<artifactId>org.openhab.core</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.openhab.core.bundles</groupId>
<artifactId>org.openhab.core.test</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>

26
bundles/org.openhab.core.semantics/src/main/java/org/openhab/core/semantics/internal/SemanticsMetadataProvider.java
View File

@ -12,8 +12,8 @@
*/
package org.openhab.core.semantics.internal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -40,6 +40,8 @@ import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Deactivate;
import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* This {@link MetadataProvider} collects semantic information about items and provides them as metadata under the
@ -56,6 +58,8 @@ import org.osgi.service.component.annotations.Reference;
public class SemanticsMetadataProvider extends AbstractProvider<Metadata>
implements ItemRegistryChangeListener, MetadataProvider {
private final Logger logger = LoggerFactory.getLogger(SemanticsMetadataProvider.class);
// the namespace to use for the metadata
public static final String NAMESPACE = "semantics";
@ -65,12 +69,7 @@ public class SemanticsMetadataProvider extends AbstractProvider<Metadata>
private final Map<List<Class<? extends Tag>>, String> propertyRelations = new HashMap<>();
// local cache of the created metadata as a map from itemName->Metadata
private final Map<String, Metadata> semantics = new TreeMap<>(new Comparator<String>() {
@Override
public int compare(String s1, String s2) {
return s1.compareTo(s2);
}
});
private final Map<String, Metadata> semantics = new TreeMap<>(String::compareTo);
private final ItemRegistry itemRegistry;
@ -106,6 +105,10 @@ public class SemanticsMetadataProvider extends AbstractProvider<Metadata>
* @param item the item to update the metadata for
*/
private void processItem(Item item) {
processItem(item, new ArrayList<>());
}
private void processItem(Item item, List<String> parentItems) {
MetadataKey key = new MetadataKey(NAMESPACE, item.getName());
Map<String, Object> configuration = new HashMap<>();
Class<? extends Tag> type = SemanticTags.getSemanticType(item);
@ -127,8 +130,15 @@ public class SemanticsMetadataProvider extends AbstractProvider<Metadata>
}
if (item instanceof GroupItem groupItem) {
parentItems.add(item.getName());
for (Item memberItem : groupItem.getMembers()) {
processItem(memberItem);
if (parentItems.contains(memberItem.getName())) {
logger.error(
"Recursive group membership found: {} is both, a direct or indirect parent and a child of {}.",
memberItem.getName(), groupItem.getName());
} else {
processItem(memberItem, parentItems);
}
}
}
}

51
bundles/org.openhab.core.semantics/src/test/java/org/openhab/core/semantics/internal/SemanticsMetadataProviderTest.java
View File

@ -36,10 +36,12 @@ import org.openhab.core.items.GroupItem;
import org.openhab.core.items.Item;
import org.openhab.core.items.ItemRegistry;
import org.openhab.core.items.Metadata;
import org.openhab.core.library.items.NumberItem;
import org.openhab.core.library.items.SwitchItem;
import org.openhab.core.semantics.ManagedSemanticTagProvider;
import org.openhab.core.semantics.SemanticTagRegistry;
import org.openhab.core.semantics.model.DefaultSemanticTagProvider;
import org.openhab.core.test.java.JavaTest;
/**
* @author Simon Lamon - Initial contribution
@ -47,7 +49,7 @@ import org.openhab.core.semantics.model.DefaultSemanticTagProvider;
@NonNullByDefault
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
public class SemanticsMetadataProviderTest {
public class SemanticsMetadataProviderTest extends JavaTest {
private static final String ITEM_NAME = "switchItem";
@ -61,6 +63,8 @@ public class SemanticsMetadataProviderTest {
@BeforeEach
public void beforeEach() throws Exception {
setupInterceptedLogger(SemanticsMetadataProvider.class, LogLevel.DEBUG);
when(managedSemanticTagProviderMock.getAll()).thenReturn(List.of());
SemanticTagRegistry semanticTagRegistry = new SemanticTagRegistryImpl(new DefaultSemanticTagProvider(),
managedSemanticTagProviderMock);
@ -235,6 +239,51 @@ public class SemanticsMetadataProviderTest {
assertNull(metadata.getConfiguration().get("hasLocation"));
}
@Test
public void testRecursiveGroupMembershipDoesNotResultInStackOverflowError() {
GroupItem groupItem1 = new GroupItem("group1");
GroupItem groupItem2 = new GroupItem("group2");
groupItem1.addMember(groupItem2);
groupItem2.addMember(groupItem1);
assertDoesNotThrow(() -> semanticsMetadataProvider.added(groupItem1));
assertLogMessage(SemanticsMetadataProvider.class, LogLevel.ERROR,
"Recursive group membership found: group1 is both, a direct or indirect parent and a child of group2.");
}
@Test
public void testIndirectRecursiveMembershipDoesNotThrowStackOverflowError() {
GroupItem groupItem1 = new GroupItem("group1");
GroupItem groupItem2 = new GroupItem("group2");
GroupItem groupItem3 = new GroupItem("group3");
groupItem1.addMember(groupItem2);
groupItem2.addMember(groupItem3);
groupItem3.addMember(groupItem1);
assertDoesNotThrow(() -> semanticsMetadataProvider.added(groupItem1));
assertLogMessage(SemanticsMetadataProvider.class, LogLevel.ERROR,
"Recursive group membership found: group1 is both, a direct or indirect parent and a child of group3.");
}
@Test
public void testDuplicateMembershipOfPlainItemsDoesNotTriggerWarning() {
GroupItem groupItem1 = new GroupItem("group1");
GroupItem groupItem2 = new GroupItem("group2");
NumberItem numberItem = new NumberItem("number");
groupItem1.addMember(groupItem2);
groupItem1.addMember(numberItem);
groupItem2.addMember(numberItem);
semanticsMetadataProvider.added(groupItem1);
assertNoLogMessage(SemanticsMetadataProvider.class);
}
private @Nullable Metadata getMetadata(Item item) {
return semanticsMetadataProvider.getAll().stream() //
.filter(metadata -> metadata.getUID().getItemName().equals(item.getName())) //