mirror of
https://github.com/danieldemus/openhab-core.git
synced 2026-07-29 12:34:22 +02:00
Rule UID syntax validation (#5467)
* Add a validity check for the rule UID A valid rule UID must consist of one or several segments separated by a colon, each segment must only contain alphanumeric, underscore or hyphen, and must not contain any other symbols. Signed-off-by: Laurent Garnier <lg.hc@free.fr> * Apply UID validation to YAML rules Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Alternatives Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Implement RulesValidator in Java instead Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Workaround to let specific errors fail rule parsing while others are just a warning Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Alternative way to express model validation expression Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Enforce rule UID in RuleRegistry Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Apply my preferred alternative Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Address review feedback Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Add tests for RuleUtil Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> * Address review comments Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> --------- Signed-off-by: Laurent Garnier <lg.hc@free.fr> Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> Co-authored-by: Laurent Garnier <lg.hc@free.fr> Co-authored-by: Ravi Nadahar <nadahar@rediffmail.com>
This commit is contained in:
Nadahar
co-authored by
Laurent Garnier
Ravi Nadahar
Laurent Garnier
Ravi Nadahar
parent
af347ff26a
commit
055259bd2e
+4
-1
@@ -21,6 +21,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
|
||||
import org.eclipse.jdt.annotation.Nullable;
|
||||
import org.openhab.core.automation.Rule;
|
||||
import org.openhab.core.automation.RuleRegistry;
|
||||
import org.openhab.core.automation.util.RuleUtil;
|
||||
import org.openhab.core.common.registry.RegistryChangeListener;
|
||||
|
||||
/**
|
||||
@@ -70,8 +71,10 @@ public class RuleSupportRuleRegistryDelegate implements RuleRegistry {
|
||||
|
||||
@Override
|
||||
public Rule add(Rule element) {
|
||||
String uid = element.getUID();
|
||||
RuleUtil.assertValidRuleUID(uid);
|
||||
ruleProvider.add(element);
|
||||
rules.add(element.getUID());
|
||||
rules.add(uid);
|
||||
|
||||
return element;
|
||||
}
|
||||
|
||||
+2
-2
@@ -63,8 +63,8 @@ public interface RuleRegistry extends Registry<Rule, String> {
|
||||
*
|
||||
* @param rule a {@link Rule} instance which have to be added into the {@link RuleRegistry}.
|
||||
* @return a copy of the added {@link Rule}
|
||||
* @throws IllegalArgumentException when a rule with the same UID already exists or some of the conditions or
|
||||
* actions has wrong format of input reference.
|
||||
* @throws IllegalArgumentException if the rule UID is invalid, a rule with the same UID already exists, or some
|
||||
* of the conditions or actions has an invalid input reference.
|
||||
* @throws IllegalStateException when the RuleManagedProvider is unavailable.
|
||||
*/
|
||||
@Override
|
||||
|
||||
+4
-1
@@ -42,6 +42,7 @@ import org.openhab.core.automation.type.ModuleTypeRegistry;
|
||||
import org.openhab.core.automation.util.ConfigurationNormalizer;
|
||||
import org.openhab.core.automation.util.ReferenceResolver;
|
||||
import org.openhab.core.automation.util.RuleBuilder;
|
||||
import org.openhab.core.automation.util.RuleUtil;
|
||||
import org.openhab.core.common.registry.AbstractRegistry;
|
||||
import org.openhab.core.common.registry.Provider;
|
||||
import org.openhab.core.common.registry.RegistryChangeListener;
|
||||
@@ -234,7 +235,8 @@ public class RuleRegistryImpl extends AbstractRegistry<Rule, String, RuleProvide
|
||||
* nor
|
||||
* in the module's module type definition.
|
||||
* @throws IllegalArgumentException
|
||||
* when a module id contains dot or when the rule with the same UID already exists.
|
||||
* If a module id contains a dot, if the rule UID is invalid, or if a rule with the
|
||||
* same UID already exists.
|
||||
*/
|
||||
@Override
|
||||
public Rule add(Rule rule) {
|
||||
@@ -459,6 +461,7 @@ public class RuleRegistryImpl extends AbstractRegistry<Rule, String, RuleProvide
|
||||
|
||||
@Override
|
||||
protected void onAddElement(Rule element) throws IllegalArgumentException {
|
||||
RuleUtil.assertValidRuleUID(element.getUID());
|
||||
try {
|
||||
resolveConfigurations(element);
|
||||
} catch (IllegalArgumentException e) {
|
||||
|
||||
+65
@@ -0,0 +1,65 @@
|
||||
/*
|
||||
* Copyright (c) 2010-2026 Contributors to the openHAB project
|
||||
*
|
||||
* See the NOTICE file(s) distributed with this work for additional
|
||||
* information.
|
||||
*
|
||||
* This program and the accompanying materials are made available under the
|
||||
* terms of the Eclipse Public License 2.0 which is available at
|
||||
* http://www.eclipse.org/legal/epl-2.0
|
||||
*
|
||||
* SPDX-License-Identifier: EPL-2.0
|
||||
*/
|
||||
package org.openhab.core.automation.util;
|
||||
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.eclipse.jdt.annotation.NonNullByDefault;
|
||||
import org.openhab.core.automation.Rule;
|
||||
|
||||
/**
|
||||
* The {@link RuleUtil} class contains utility methods for {@link Rule} objects.
|
||||
*
|
||||
* This class cannot be instantiated, it only contains static methods.
|
||||
*
|
||||
* @author Ravi Nadahar - Initial contribution
|
||||
*/
|
||||
@NonNullByDefault
|
||||
public class RuleUtil {
|
||||
|
||||
private static final Pattern RULE_UID_PATTERN = Pattern.compile(
|
||||
"[^\\s/\\\\\\x00-\\x1F\\x7F](?:[^/\\\\\\x00-\\x08\\x0A-\\x1F\\x7F]*[^\\s/\\\\\\x00-\\x1F\\x7F])?",
|
||||
Pattern.UNICODE_CHARACTER_CLASS);
|
||||
|
||||
/**
|
||||
* Returns {@code true} if the specified UID is a valid rule UID, otherwise {@code false}.
|
||||
* <p>
|
||||
* A valid rule UID is any string that doesn't contain {@code /}, {@code \} and has no leading or trailing
|
||||
* whitespace.
|
||||
*
|
||||
* @param uid the UID of the rule to be checked.
|
||||
* @return {@code true} if the specified UID is a valid rule UID, {@code false} otherwise.
|
||||
*/
|
||||
public static boolean isValidRuleUID(final String uid) {
|
||||
return RULE_UID_PATTERN.matcher(uid).matches();
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensures that the specified rule UID is valid.
|
||||
* <p>
|
||||
* If the rule UID is invalid an {@link IllegalArgumentException} is thrown, otherwise this method returns
|
||||
* silently.
|
||||
* <p>
|
||||
* A valid rule UID is any string that doesn't contain {@code /}, {@code \} and has no leading or trailing
|
||||
* whitespace.
|
||||
*
|
||||
* @param uid the UID of the rule to be checked.
|
||||
* @throws IllegalArgumentException If the specified rule UID is invalid.
|
||||
*/
|
||||
public static void assertValidRuleUID(String uid) throws IllegalArgumentException {
|
||||
if (!isValidRuleUID(uid)) {
|
||||
throw new IllegalArgumentException("The specified UID of rule '" + uid
|
||||
+ "' is invalid! A rule UID can't contain '/', '\\' or have leading or trailing whitespace.");
|
||||
}
|
||||
}
|
||||
}
|
||||
+68
@@ -0,0 +1,68 @@
|
||||
/*
|
||||
* Copyright (c) 2010-2026 Contributors to the openHAB project
|
||||
*
|
||||
* See the NOTICE file(s) distributed with this work for additional
|
||||
* information.
|
||||
*
|
||||
* This program and the accompanying materials are made available under the
|
||||
* terms of the Eclipse Public License 2.0 which is available at
|
||||
* http://www.eclipse.org/legal/epl-2.0
|
||||
*
|
||||
* SPDX-License-Identifier: EPL-2.0
|
||||
*/
|
||||
package org.openhab.core.automation.util;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
import static org.openhab.core.automation.util.RuleUtil.*;
|
||||
|
||||
import org.eclipse.jdt.annotation.NonNullByDefault;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
/**
|
||||
* @author Ravi Nadahar - Initial contribution
|
||||
*/
|
||||
@NonNullByDefault
|
||||
public class RuleUtilTest {
|
||||
|
||||
@Test
|
||||
public void testIsValidRuleUID() {
|
||||
assertFalse(isValidRuleUID(""));
|
||||
assertTrue(isValidRuleUID("a"));
|
||||
assertTrue(isValidRuleUID("a b"));
|
||||
assertFalse(isValidRuleUID("a\nb"));
|
||||
assertTrue(isValidRuleUID("a\tb"));
|
||||
assertTrue(isValidRuleUID("OH™"));
|
||||
assertFalse(isValidRuleUID("/OH™"));
|
||||
assertFalse(isValidRuleUID("\\OH™"));
|
||||
assertFalse(isValidRuleUID("OH/™"));
|
||||
assertFalse(isValidRuleUID("OH\\™"));
|
||||
assertFalse(isValidRuleUID("OH™/"));
|
||||
assertFalse(isValidRuleUID("OH™\\"));
|
||||
assertTrue(isValidRuleUID("rule:xania:❣⟺⌘:3"));
|
||||
assertFalse(isValidRuleUID("rule:xania:❣⟺⌘:3\t"));
|
||||
assertFalse(isValidRuleUID("\nnope"));
|
||||
assertFalse(isValidRuleUID(" a"));
|
||||
assertFalse(isValidRuleUID("a "));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAssertValidRuleUID() {
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID(""));
|
||||
assertDoesNotThrow(() -> assertValidRuleUID("a"));
|
||||
assertDoesNotThrow(() -> assertValidRuleUID("a b"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("a\nb"));
|
||||
assertDoesNotThrow(() -> assertValidRuleUID("a\tb"));
|
||||
assertDoesNotThrow(() -> assertValidRuleUID("OH™"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("/OH™"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("\\OH™"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("OH/™"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("OH\\™"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("OH™/"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("OH™\\"));
|
||||
assertDoesNotThrow(() -> assertValidRuleUID("rule:xania:❣⟺⌘:3"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("rule:xania:❣⟺⌘:3\t"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("\nnope"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID(" a"));
|
||||
assertThrows(IllegalArgumentException.class, () -> assertValidRuleUID("a "));
|
||||
}
|
||||
}
|
||||
+23
-6
@@ -23,6 +23,7 @@ import java.text.MessageFormat;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.CopyOnWriteArrayList;
|
||||
@@ -38,6 +39,7 @@ import org.eclipse.jdt.annotation.Nullable;
|
||||
import org.eclipse.xtext.resource.SynchronizedXtextResourceSet;
|
||||
import org.eclipse.xtext.resource.XtextResource;
|
||||
import org.eclipse.xtext.resource.XtextResourceSet;
|
||||
import org.eclipse.xtext.validation.AbstractValidationDiagnostic;
|
||||
import org.openhab.core.model.core.EventType;
|
||||
import org.openhab.core.model.core.ModelRepository;
|
||||
import org.openhab.core.model.core.ModelRepositoryChangeListener;
|
||||
@@ -343,15 +345,30 @@ public class ModelRepositoryImpl implements ModelRepository {
|
||||
|
||||
// Check for validation errors, but log them only
|
||||
try {
|
||||
String modelType = resource.getURI().fileExtension();
|
||||
String modelType = resource.getURI().fileExtension().toLowerCase(Locale.ROOT);
|
||||
final org.eclipse.emf.common.util.Diagnostic diagnostic = safeEmf
|
||||
.call(() -> Diagnostician.INSTANCE.validate(resource.getContents().getFirst()));
|
||||
for (org.eclipse.emf.common.util.Diagnostic d : diagnostic.getChildren()) {
|
||||
if (d.getSeverity() == org.eclipse.emf.common.util.Diagnostic.ERROR
|
||||
&& !"rules".equalsIgnoreCase(modelType) && !"script".equalsIgnoreCase(modelType)) {
|
||||
errors.add(d.getMessage());
|
||||
} else {
|
||||
warnings.add(d.getMessage());
|
||||
switch (modelType) {
|
||||
case "rules":
|
||||
if (d instanceof AbstractValidationDiagnostic vd
|
||||
&& d.getSeverity() == org.eclipse.emf.common.util.Diagnostic.ERROR
|
||||
&& "uid".equals(vd.getIssueCode())) {
|
||||
errors.add(d.getMessage());
|
||||
} else {
|
||||
warnings.add(d.getMessage());
|
||||
}
|
||||
break;
|
||||
case "script":
|
||||
warnings.add(d.getMessage());
|
||||
break;
|
||||
default:
|
||||
if (d.getSeverity() == org.eclipse.emf.common.util.Diagnostic.ERROR) {
|
||||
errors.add(d.getMessage());
|
||||
} else {
|
||||
warnings.add(d.getMessage());
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!errors.isEmpty()) {
|
||||
|
||||
@@ -14,6 +14,7 @@ Export-Package: org.openhab.core.model.rule,\
|
||||
Import-Package: \
|
||||
org.openhab.core.automation,\
|
||||
org.openhab.core.automation.module.script.rulesupport.shared,\
|
||||
org.openhab.core.automation.util,\
|
||||
org.openhab.core.common,\
|
||||
org.openhab.core.common.registry,\
|
||||
org.openhab.core.events,\
|
||||
|
||||
+1
-1
@@ -58,7 +58,7 @@ Workflow {
|
||||
generateStub = false
|
||||
}
|
||||
validator = {
|
||||
generateStub = false
|
||||
generateXtendStub = false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+58
@@ -0,0 +1,58 @@
|
||||
/*
|
||||
* Copyright (c) 2010-2026 Contributors to the openHAB project
|
||||
*
|
||||
* See the NOTICE file(s) distributed with this work for additional
|
||||
* information.
|
||||
*
|
||||
* This program and the accompanying materials are made available under the
|
||||
* terms of the Eclipse Public License 2.0 which is available at
|
||||
* http://www.eclipse.org/legal/epl-2.0
|
||||
*
|
||||
* SPDX-License-Identifier: EPL-2.0
|
||||
*/
|
||||
package org.openhab.core.model.rule.validation;
|
||||
|
||||
import org.eclipse.emf.ecore.EObject;
|
||||
import org.eclipse.jdt.annotation.NonNullByDefault;
|
||||
import org.eclipse.jdt.annotation.Nullable;
|
||||
import org.eclipse.xtext.nodemodel.ICompositeNode;
|
||||
import org.eclipse.xtext.nodemodel.util.NodeModelUtils;
|
||||
import org.eclipse.xtext.validation.Check;
|
||||
import org.openhab.core.automation.util.RuleUtil;
|
||||
import org.openhab.core.model.rule.rules.Rule;
|
||||
import org.openhab.core.model.rule.rules.RulesPackage;
|
||||
|
||||
/**
|
||||
* This class contains custom validation rules.
|
||||
*
|
||||
* See https://www.eclipse.org/Xtext/documentation/303_runtime_concepts.html#validation
|
||||
*
|
||||
* @author Ravi Nadahar - Initial contribution
|
||||
*/
|
||||
@NonNullByDefault
|
||||
public class RulesValidator extends AbstractRulesValidator {
|
||||
|
||||
@Check
|
||||
public void checkRuleUID(@Nullable Rule rule) {
|
||||
String uid;
|
||||
if (rule == null || (uid = rule.getUid()) == null) {
|
||||
return;
|
||||
}
|
||||
if (!RuleUtil.isValidRuleUID(uid)) {
|
||||
error(buildMsgWithLineNb(rule, "Rule UID '" + uid
|
||||
+ "' is invalid. A rule UID can't contain '/', '\\' or have leading or trailing whitespace."), rule,
|
||||
RulesPackage.Literals.RULE__UID, "uid");
|
||||
}
|
||||
}
|
||||
|
||||
private static String buildMsgWithLineNb(EObject object, String msg) {
|
||||
ICompositeNode node = NodeModelUtils.getNode(object);
|
||||
if (node == null) {
|
||||
return msg;
|
||||
}
|
||||
int startLine = node.getStartLine();
|
||||
int endLine = node.getEndLine();
|
||||
return (startLine == endLine) ? "Line " + startLine + ": " + msg
|
||||
: "Line " + startLine + "-" + endLine + ": " + msg;
|
||||
}
|
||||
}
|
||||
+9
-12
@@ -17,11 +17,11 @@ import java.util.HashSet;
|
||||
import java.util.Iterator;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.Map.Entry;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.eclipse.jdt.annotation.NonNull;
|
||||
import org.eclipse.jdt.annotation.Nullable;
|
||||
@@ -31,7 +31,7 @@ import org.openhab.core.automation.Rule;
|
||||
import org.openhab.core.automation.Rule.TemplateState;
|
||||
import org.openhab.core.automation.Trigger;
|
||||
import org.openhab.core.automation.Visibility;
|
||||
import org.openhab.core.common.AbstractUID;
|
||||
import org.openhab.core.automation.util.RuleUtil;
|
||||
import org.openhab.core.config.core.ConfigDescriptionParameter;
|
||||
import org.openhab.core.io.dto.ModularDTO;
|
||||
import org.openhab.core.io.dto.SerializationException;
|
||||
@@ -53,8 +53,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode;
|
||||
@YamlElementName("rules")
|
||||
public class YamlRuleDTO implements ModularDTO<YamlRuleDTO, ObjectMapper, JsonNode>, YamlElement, Cloneable {
|
||||
|
||||
protected static final Pattern UID_SEGMENT_PATTERN = Pattern.compile("[a-zA-Z0-9_][a-zA-Z0-9_-]*");
|
||||
|
||||
public String uid;
|
||||
public String template;
|
||||
public TemplateState templateState;
|
||||
@@ -270,14 +268,13 @@ public class YamlRuleDTO implements ModularDTO<YamlRuleDTO, ObjectMapper, JsonNo
|
||||
return false;
|
||||
}
|
||||
boolean ok = true;
|
||||
// Check that uid only contains valid characters
|
||||
String[] segments = uid.split(AbstractUID.SEPARATOR);
|
||||
for (String segment : segments) {
|
||||
if (!UID_SEGMENT_PATTERN.matcher(segment).matches()) {
|
||||
addToList(errors, "invalid rule \"%s\": segment \"%s\" in the uid doesn't match the expected syntax %s"
|
||||
.formatted(uid, segment, UID_SEGMENT_PATTERN.pattern()));
|
||||
ok = false;
|
||||
}
|
||||
|
||||
// Validate the UID
|
||||
if (!RuleUtil.isValidRuleUID(uid)) {
|
||||
addToList(errors, String.format(Locale.ROOT,
|
||||
"invalid rule \"%s\": UID is invalid. A rule UID can't contain '/', '\\' or have leading or trailing whitespace.",
|
||||
uid));
|
||||
ok = false;
|
||||
}
|
||||
|
||||
// Check that name is present
|
||||
|
||||
+2
-2
@@ -94,13 +94,13 @@ public class YamlRuleDTOTest {
|
||||
rule.uid = "rule:id";
|
||||
assertTrue(rule.isValid(null, null));
|
||||
|
||||
rule.uid = "rule:type:@id";
|
||||
rule.uid = "rule:type:/id";
|
||||
assertFalse(rule.isValid(null, null));
|
||||
|
||||
rule.uid = "rule:type:id";
|
||||
assertTrue(rule.isValid(null, null));
|
||||
|
||||
rule.uid = "rule:type:$subType:id";
|
||||
rule.uid = "rule:type:\\subType:id";
|
||||
assertFalse(rule.isValid(null, null));
|
||||
|
||||
rule.uid = "rule:type:subType:id";
|
||||
|
||||
Reference in New Issue
Block a user