Let wifi own portal teardown on provisioning close, build the redirect with buf_append_printf, trim comments

esphome/components/captive_portal/captive_portal.cpp

@@ -6,9 +6,6 @@
 #include "esphome/core/string_ref.h"
 #include "esphome/components/wifi/scan_list.h"
 #include "esphome/components/wifi/wifi_component.h"
-#ifdef USE_PROVISIONING
-#include "esphome/components/provisioning/provisioning.h"
-#endif
 #include "captive_index.h"
 
 namespace esphome::captive_portal {
@@ -81,20 +78,6 @@ void CaptivePortal::handle_wifisave(AsyncWebServerRequest *request) {
 void CaptivePortal::setup() {
   // Disable loop by default - will be enabled when captive portal starts
   this->disable_loop();
-#ifdef USE_PROVISIONING
-  // The captive portal is a provisioning surface: once the provisioning window
-  // has closed, stop serving it. WiFi's own closed-callback shuts down the
-  // access point the portal runs on, and the gated fallback in WiFiComponent's
-  // loop() ensures neither is started again afterwards.
-  if (provisioning::global_provisioning_manager != nullptr) {
-    provisioning::global_provisioning_manager->add_on_closed_callback([this]() {
-      if (this->active_) {
-        ESP_LOGD(TAG, "Provisioning window closed; stopping captive portal");
-        this->end();
-      }
-    });
-  }
-#endif
 }
 void CaptivePortal::start() {
   this->base_->init();

esphome/components/web_server/__init__.py

@@ -54,12 +54,9 @@ _LOGGER = logging.getLogger(__name__)
 
 
 def AUTO_LOAD() -> list[str]:
-    # No config parameter on purpose: that would make this a late (dynamic) auto-load and
-    # ota.web_server's dependency on web_server_base would not be satisfied in time.
+    # No config parameter: a dynamic auto-load would satisfy ota.web_server's dependency too late
     auto_load = ["json", "web_server_base"]
-    # The AP mode DNS server (web_server_base/dns_server_esp32_idf) uses socket; only
-    # configs with a WiFi access point can end up in AP mode. CORE.raw_config is set
-    # after package merging, so a wifi block from a package is visible here.
+    # The AP mode DNS server needs socket; CORE.raw_config already has a wifi block from a package
     wifi = CORE.raw_config.get(CONF_WIFI) if CORE.raw_config else None
     if (
         CORE.is_esp32
@@ -376,10 +373,8 @@ def serve_local(config: ConfigType, wifi_config: ConfigType | None) -> bool:
 
 
 def serve_captive(config: ConfigType, full_config: ConfigType) -> bool:
-    """web_server runs its own captive portal while the AP is up: embedded interface plus
-    an access point, unless captive_portal (which owns that role) is configured. Only on
-    port 80: the OS captive portal probes and the DHCP portal URI always use port 80, so
-    a portal on another port could never be discovered."""
+    """Serve the embedded interface as a captive portal while the AP is up, unless captive_portal
+    owns that role. Port 80 only: the OS probes and the DHCP portal URI never use another port."""
     wifi_config = full_config.get(CONF_WIFI)
     return (
         "captive_portal" not in full_config
@@ -395,6 +390,7 @@ def _final_validate_ap_mode(config: ConfigType) -> None:
     wifi_config = full_config.get(CONF_WIFI)
     captive = serve_captive(config, full_config)
     local = serve_local(config, wifi_config)
+    ap_only = wifi_is_ap_only(wifi_config)
     if captive:
         web_server_base.consume_captive_dns_sockets(config, "web_server")
     # Surface behavior that the config does not spell out.
@@ -410,9 +406,9 @@ def _final_validate_ap_mode(config: ConfigType) -> None:
     elif captive:
         _LOGGER.info(
             "web_server will act as a captive portal while the %saccess point is active.",
-            "" if wifi_is_ap_only(wifi_config) else "fallback ",
+            "" if ap_only else "fallback ",
         )
-    if not wifi_is_ap_only(wifi_config):
+    if not ap_only:
         return
     if not local:
         _LOGGER.warning(

esphome/components/web_server/web_server.cpp

@@ -440,7 +440,7 @@ void WebServer::loop() {
 
 #ifdef USE_WEBSERVER_CAPTIVE
 void WebServer::start_captive() {
-  // CaptiveDNS::start() no-ops too; this guard just avoids repeating the log and enable_loop
+  // Avoid repeating the log and enable_loop on a second start
   if (this->dns_.is_running())
     return;
   network::IPAddress ip = wifi::global_wifi_component->wifi_soft_ap_ip();
@@ -456,15 +456,14 @@ void WebServer::start_captive() {
 void WebServer::end_captive() { this->dns_.stop(); }
 
 void WebServer::handle_not_found_(AsyncWebServerRequest *request) {
-  // OS captive portal probe (or any other unknown page) while the AP is up: send the browser
-  // to the real page. A redirect rather than the page itself, because the interface resolves
-  // its /events and REST paths relative to the page URL.
+  // OS captive portal probe while the AP is up: a redirect rather than the page itself, because
+  // the interface resolves its /events and REST paths relative to the page URL.
   if (this->dns_.is_running() && request->method() == HTTP_GET) {
-    // Captive mode requires port 80 (enforced at validation), so no port suffix is needed.
+    char ip_buf[network::IP_ADDRESS_BUFFER_SIZE];
+    wifi::global_wifi_component->wifi_soft_ap_ip().str_to(ip_buf);
+    // Captive mode requires port 80 (enforced at validation), so no port suffix
     char location[7 + network::IP_ADDRESS_BUFFER_SIZE + 1];
-    size_t pos = buf_append_str(location, sizeof(location), 0, "http://");
-    wifi::global_wifi_component->wifi_soft_ap_ip().str_to(location + pos);
-    buf_append_str(location, sizeof(location), strlen(location), "/");
+    buf_append_printf(location, sizeof(location), 0, "http://%s/", ip_buf);
     request->redirect(location);
     return;
   }

esphome/components/web_server/web_server.h

@@ -4,9 +4,7 @@
 
 #include "esphome/components/json/json_util.h"
 #include "esphome/components/web_server_base/web_server_base.h"
-#ifdef USE_WEBSERVER_CAPTIVE
 #include "esphome/components/web_server_base/captive_dns.h"
-#endif
 #ifdef USE_WEBSERVER
 #include "esphome/core/component.h"
 #include "esphome/core/entity_base.h"
@@ -281,12 +279,8 @@ class WebServer final : public Component, public AsyncWebHandler {
   void handle_index_request(AsyncWebServerRequest *request);
 
 #ifdef USE_WEBSERVER_CAPTIVE
-  /** AP mode: run a DNS server that answers every name with the AP address and redirect any
-   * unknown URL to the interface, so a phone joining the AP opens it through the OS captive
-   * portal check. Started and ended by the wifi component with the access point. start may run
-   * before setup() (wifi sets up first): safe because enable_loop() is a no-op before setup;
-   * nothing but the DNS server may be touched, in particular not base_ or the handlers.
-   */
+  /// AP mode: DNS answers every name with the AP address and unknown URLs redirect to the interface.
+  /// wifi calls start before setup(), so it may touch nothing but dns_ (enable_loop is a no-op then).
   void start_captive();
   void end_captive();
   bool is_captive() const { return this->dns_.is_running(); }

esphome/components/web_server_base/dns_server_esp32_idf.h

@@ -1,8 +1,6 @@
 #pragma once
 #include "esphome/core/defines.h"
-// Small DNS server that answers every query with the access point address, so a
-// phone joining the AP opens the captive portal or web_server page on its own.
-// Shared by captive_portal and the web_server AP mode.
+// ESP32 backend for CaptiveDNS: answers every A query with the access point address.
 #if defined(USE_ESP32) && (defined(USE_CAPTIVE_PORTAL) || defined(USE_WEBSERVER_CAPTIVE))
 
 #include "esphome/core/helpers.h"

esphome/components/wifi/wifi_component.cpp

@@ -637,15 +637,14 @@ void WiFiComponent::setup() {
 #endif
 
 #if defined(USE_PROVISIONING) && defined(USE_WIFI_AP)
-  // The access point is a provisioning surface: once the provisioning window has
-  // closed, end both portals and shut it down (mirrors the teardown done on a
-  // successful connection). The fallback block in loop() is gated so none of them
-  // is started again afterwards.
+  // The access point and the portals on it are provisioning surfaces: once the window has
+  // closed, end them (mirrors the teardown on a successful connection). The fallback block in
+  // loop() is gated so none of them is started again afterwards.
   if (provisioning::global_provisioning_manager != nullptr) {
     provisioning::global_provisioning_manager->add_on_closed_callback([this]() {
+      this->end_ap_portal_();  // no-op when nothing is active, so not tied to ap_setup_
       if (this->ap_setup_) {
         ESP_LOGD(TAG, "Provisioning window closed; disabling AP");
-        this->end_ap_portal_();
         this->wifi_mode_({}, false);
       }
     });
@@ -1638,9 +1637,7 @@ void WiFiComponent::check_connecting_finished(uint32_t now) {
     this->retry_phase_ = WiFiRetryPhase::INITIAL_CONNECT;
     this->num_retried_ = 0;
     if (this->has_ap()) {
-#ifdef USE_WIFI_AP
       this->end_ap_portal_();
-#endif
       ESP_LOGD(TAG, "Disabling AP");
       this->wifi_mode_({}, false);
     }
@@ -2237,13 +2234,11 @@ bool WiFiComponent::is_ap_portal_active_() {
   return this->is_captive_portal_active_();
 }
 
-#ifdef USE_WIFI_AP
-// global_web_server needs no null check: codegen always instantiates WebServer when
-// USE_WEBSERVER_CAPTIVE is defined, and the constructor assigns the global.
+// Neither global needs a null check: codegen always instantiates the component when its
+// define is set, and the constructor assigns the global.
 void WiFiComponent::start_ap_portal_() {
 #ifdef USE_CAPTIVE_PORTAL
-  if (captive_portal::global_captive_portal != nullptr)
-    captive_portal::global_captive_portal->start();
+  captive_portal::global_captive_portal->start();
 #endif
 #ifdef USE_WEBSERVER_CAPTIVE
   web_server::global_web_server->start_captive();
@@ -2259,7 +2254,6 @@ void WiFiComponent::end_ap_portal_() {
   web_server::global_web_server->end_captive();
 #endif
 }
-#endif  // USE_WIFI_AP
 bool WiFiComponent::is_improv_ble_active_() {
 #ifdef USE_IMPROV_BLE
   return improv_ble::global_improv_component != nullptr && improv_ble::global_improv_component->is_active();

esphome/components/wifi/wifi_component.h

@@ -799,10 +799,8 @@ class WiFiComponent final : public Component {
   bool is_captive_portal_active_();
   /// captive_portal or the web_server AP mode is serving a user on the access point
   bool is_ap_portal_active_();
-#ifdef USE_WIFI_AP
   void start_ap_portal_();
   void end_ap_portal_();
-#endif
   bool is_improv_ble_active_();
 
 #ifdef USE_WIFI_FAST_CONNECT

esphome/components/wifi/wifi_component_esp_idf.cpp

@@ -1164,26 +1164,17 @@ bool WiFiComponent::wifi_ap_ip_config_(const optional<ManualIP> &manual_ip) {
   }
 
 #if (defined(USE_CAPTIVE_PORTAL) || defined(USE_WEBSERVER_CAPTIVE)) && ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 0)
-  // Configure DHCP Option 114 (Captive Portal URI) if captive portal or the web_server AP
-  // mode is enabled. This provides a standards-compliant way for clients to discover the portal
-#ifdef USE_WEBSERVER_CAPTIVE
-  // web_server AP mode always serves the portal when compiled in
-  const bool has_portal = true;
-#else
-  const bool has_portal = captive_portal::global_captive_portal != nullptr;
-#endif
-  if (has_portal) {
-    // Buffer must be static - dhcps_set_option_info stores pointer, doesn't copy
-    static char captive_portal_uri[24];        // "http://" (7) + IPv4 max (15) + null
-    memcpy(captive_portal_uri, "http://", 7);  // NOLINT(bugprone-not-null-terminated-result) - str_to null-terminates
-    network::IPAddress(&info.ip).str_to(captive_portal_uri + 7);
-    err = esp_netif_dhcps_option(s_ap_netif, ESP_NETIF_OP_SET, ESP_NETIF_CAPTIVEPORTAL_URI, captive_portal_uri,
-                                 strlen(captive_portal_uri));
-    if (err != ESP_OK) {
-      ESP_LOGV(TAG, "Failed to set DHCP captive portal URI: %s", esp_err_to_name(err));
-    } else {
-      ESP_LOGV(TAG, "DHCP Captive Portal URI set to: %s", captive_portal_uri);
-    }
+  // DHCP option 114 (captive portal URI) lets clients find the portal; either define means one exists
+  // Buffer must be static - dhcps_set_option_info stores pointer, doesn't copy
+  static char captive_portal_uri[24];        // "http://" (7) + IPv4 max (15) + null
+  memcpy(captive_portal_uri, "http://", 7);  // NOLINT(bugprone-not-null-terminated-result) - str_to null-terminates
+  network::IPAddress(&info.ip).str_to(captive_portal_uri + 7);
+  err = esp_netif_dhcps_option(s_ap_netif, ESP_NETIF_OP_SET, ESP_NETIF_CAPTIVEPORTAL_URI, captive_portal_uri,
+                               strlen(captive_portal_uri));
+  if (err != ESP_OK) {
+    ESP_LOGV(TAG, "Failed to set DHCP captive portal URI: %s", esp_err_to_name(err));
+  } else {
+    ESP_LOGV(TAG, "DHCP Captive Portal URI set to: %s", captive_portal_uri);
   }
 #endif