Limit hostnames to 31 characters (#2531)

esphome/config_validation.py

@@ -903,21 +903,9 @@ def validate_bytes(value):
 
 def hostname(value):
     value = string(value)
-    warned_underscore = False
+    if re.match(r"^[a-z0-9-]{1,63}$", value, re.IGNORECASE) is not None:
-    if len(value) > 63:
+        return value
-        raise Invalid("Hostnames can only be 63 characters long")
+    raise Invalid(f"Invalid hostname: {value}")
-    for c in value:
-        if not (c.isalnum() or c in "-_"):
-            raise Invalid("Hostname can only have alphanumeric characters and -")
-        if c in "_" and not warned_underscore:
-            _LOGGER.warning(
-                "'%s': Using the '_' (underscore) character in the hostname is discouraged "
-                "as it can cause problems with some DHCP and local name services. "
-                "For more information, see https://esphome.io/guides/faq.html#why-shouldn-t-i-use-underscores-in-my-device-name",
-                value,
-            )
-            warned_underscore = True
-    return value
 
 
 def domain(value):

esphome/core/config.py

@@ -55,6 +55,24 @@ CONF_NAME_ADD_MAC_SUFFIX = "name_add_mac_suffix"
 VALID_INCLUDE_EXTS = {".h", ".hpp", ".tcc", ".ino", ".cpp", ".c"}
 
 
+def validate_hostname(config):
+    max_length = 31
+    if config[CONF_NAME_ADD_MAC_SUFFIX]:
+        max_length -= 7  # "-AABBCC" is appended when add mac suffix option is used
+    if len(config[CONF_NAME]) > max_length:
+        raise cv.Invalid(
+            f"Hostnames can only be {max_length} characters long", path=[CONF_NAME]
+        )
+    if "_" in config[CONF_NAME]:
+        _LOGGER.warning(
+            "'%s': Using the '_' (underscore) character in the hostname is discouraged "
+            "as it can cause problems with some DHCP and local name services. "
+            "For more information, see https://esphome.io/guides/faq.html#why-shouldn-t-i-use-underscores-in-my-device-name",
+            config[CONF_NAME],
+        )
+    return config
+
+
 def valid_include(value):
     try:
         return cv.directory(value)
@@ -79,42 +97,47 @@ def valid_project_name(value: str):
 
 
 CONF_ESP8266_RESTORE_FROM_FLASH = "esp8266_restore_from_flash"
-CONFIG_SCHEMA = cv.Schema(
+CONFIG_SCHEMA = cv.All(
-    {
+    cv.Schema(
-        cv.Required(CONF_NAME): cv.hostname,
+        {
-        cv.Optional(CONF_COMMENT): cv.string,
+            cv.Required(CONF_NAME): cv.valid_name,
-        cv.Required(CONF_BUILD_PATH): cv.string,
+            cv.Optional(CONF_COMMENT): cv.string,
-        cv.Optional(CONF_PLATFORMIO_OPTIONS, default={}): cv.Schema(
+            cv.Required(CONF_BUILD_PATH): cv.string,
-            {
+            cv.Optional(CONF_PLATFORMIO_OPTIONS, default={}): cv.Schema(
-                cv.string_strict: cv.Any([cv.string], cv.string),
+                {
-            }
+                    cv.string_strict: cv.Any([cv.string], cv.string),
-        ),
+                }
-        cv.Optional(CONF_ON_BOOT): automation.validate_automation(
+            ),
-            {
+            cv.Optional(CONF_ON_BOOT): automation.validate_automation(
-                cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(StartupTrigger),
+                {
-                cv.Optional(CONF_PRIORITY, default=600.0): cv.float_,
+                    cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(StartupTrigger),
-            }
+                    cv.Optional(CONF_PRIORITY, default=600.0): cv.float_,
-        ),
+                }
-        cv.Optional(CONF_ON_SHUTDOWN): automation.validate_automation(
+            ),
-            {
+            cv.Optional(CONF_ON_SHUTDOWN): automation.validate_automation(
-                cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(ShutdownTrigger),
+                {
-            }
+                    cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(ShutdownTrigger),
-        ),
+                }
-        cv.Optional(CONF_ON_LOOP): automation.validate_automation(
+            ),
-            {
+            cv.Optional(CONF_ON_LOOP): automation.validate_automation(
-                cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(LoopTrigger),
+                {
-            }
+                    cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(LoopTrigger),
-        ),
+                }
-        cv.Optional(CONF_INCLUDES, default=[]): cv.ensure_list(valid_include),
+            ),
-        cv.Optional(CONF_LIBRARIES, default=[]): cv.ensure_list(cv.string_strict),
+            cv.Optional(CONF_INCLUDES, default=[]): cv.ensure_list(valid_include),
-        cv.Optional(CONF_NAME_ADD_MAC_SUFFIX, default=False): cv.boolean,
+            cv.Optional(CONF_LIBRARIES, default=[]): cv.ensure_list(cv.string_strict),
-        cv.Optional(CONF_PROJECT): cv.Schema(
+            cv.Optional(CONF_NAME_ADD_MAC_SUFFIX, default=False): cv.boolean,
-            {
+            cv.Optional(CONF_PROJECT): cv.Schema(
-                cv.Required(CONF_NAME): cv.All(cv.string_strict, valid_project_name),
+                {
-                cv.Required(CONF_VERSION): cv.string_strict,
+                    cv.Required(CONF_NAME): cv.All(
-            }
+                        cv.string_strict, valid_project_name
-        ),
+                    ),
-    }
+                    cv.Required(CONF_VERSION): cv.string_strict,
+                }
+            ),
+        }
+    ),
+    validate_hostname,
 )
 
 PRELOAD_CONFIG_SCHEMA = cv.Schema(

tests/unit_tests/test_config_validation.py

@@ -40,28 +40,6 @@ def test_valid_name__invalid(value):
         config_validation.valid_name(value)
 
 
-@pytest.mark.parametrize("value", ("foo", "bar123", "foo-bar"))
-def test_hostname__valid(value):
-    actual = config_validation.hostname(value)
-
-    assert actual == value
-
-
-@pytest.mark.parametrize("value", ("foo bar", "foobar ", "foo#bar"))
-def test_hostname__invalid(value):
-    with pytest.raises(Invalid):
-        config_validation.hostname(value)
-
-
-def test_hostname__warning(caplog):
-    actual = config_validation.hostname("foo_bar")
-    assert actual == "foo_bar"
-    assert (
-        "Using the '_' (underscore) character in the hostname is discouraged"
-        in caplog.text
-    )
-
-
 @given(one_of(integers(), text()))
 def test_string__valid(value):
     actual = config_validation.string(value)