Movable HTTPClient and fixing WiFiClient copy

libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp

@@ -28,6 +28,16 @@
 #include <StreamDev.h>
 #include <base64.h>
 
+// per https://github.com/esp8266/Arduino/issues/8231
+// make sure HTTPClient can be utilized as a movable class member
+static_assert(std::is_default_constructible_v<HTTPClient>, "");
+static_assert(!std::is_copy_constructible_v<HTTPClient>, "");
+static_assert(std::is_move_constructible_v<HTTPClient>, "");
+static_assert(std::is_move_assignable_v<HTTPClient>, "");
+
+static const char defaultUserAgentPstr[] PROGMEM = "ESP8266HTTPClient";
+const String HTTPClient::defaultUserAgent = defaultUserAgentPstr;
+
 static int StreamReportToHttpClientReport (Stream::Report streamSendError)
 {
     switch (streamSendError)
@@ -41,27 +51,6 @@ static int StreamReportToHttpClientReport (Stream::Report streamSendError)
     return 0; // never reached, keep gcc quiet
 }
 
-/**
- * constructor
- */
-HTTPClient::HTTPClient()
-    : _client(nullptr), _userAgent(F("ESP8266HTTPClient"))
-{
-}
-
-/**
- * destructor
- */
-HTTPClient::~HTTPClient()
-{
-    if(_client) {
-        _client->stop();
-    }
-    if(_currentHeaders) {
-        delete[] _currentHeaders;
-    }
-}
-
 void HTTPClient::clear()
 {
     _returnCode = 0;
@@ -445,7 +434,7 @@ int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t s
         }
 
         // transfer all of it, with send-timeout
-        if (size && StreamConstPtr(payload, size).sendAll(_client) != size)
+        if (size && StreamConstPtr(payload, size).sendAll(_client.get()) != size)
             return returnError(HTTPC_ERROR_SEND_PAYLOAD_FAILED);
 
         // handle Server Response (Header)
@@ -546,7 +535,7 @@ int HTTPClient::sendRequest(const char * type, Stream * stream, size_t size)
     }
 
     // transfer all of it, with timeout
-    size_t transferred = stream->sendSize(_client, size);
+    size_t transferred = stream->sendSize(_client.get(), size);
     if (transferred != size)
     {
         DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] short write, asked for %d but got %d failed.\n", size, transferred);
@@ -596,7 +585,7 @@ WiFiClient& HTTPClient::getStream(void)
 WiFiClient* HTTPClient::getStreamPtr(void)
 {
     if(connected()) {
-        return _client;
+        return _client.get();
     }
 
     DEBUG_HTTPCLIENT("[HTTP-Client] getStreamPtr: not connected\n");
@@ -791,10 +780,7 @@ void HTTPClient::addHeader(const String& name, const String& value, bool first,
 void HTTPClient::collectHeaders(const char* headerKeys[], const size_t headerKeysCount)
 {
     _headerKeysCount = headerKeysCount;
-    if(_currentHeaders) {
-        delete[] _currentHeaders;
-    }
-    _currentHeaders = new RequestArgument[_headerKeysCount];
+    _currentHeaders = std::make_unique<RequestArgument[]>(_headerKeysCount);
     for(size_t i = 0; i < _headerKeysCount; i++) {
         _currentHeaders[i].key = headerKeys[i];
     }
@@ -936,7 +922,7 @@ bool HTTPClient::sendHeader(const char * type)
     DEBUG_HTTPCLIENT("[HTTP-Client] sending request header\n-----\n%s-----\n", header.c_str());
 
     // transfer all of it, with timeout
-    return StreamConstPtr(header).sendAll(_client) == header.length();
+    return StreamConstPtr(header).sendAll(_client.get()) == header.length();
 }
 
 /**

libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.h

@@ -26,11 +26,12 @@
 #ifndef ESP8266HTTPClient_H_
 #define ESP8266HTTPClient_H_
 
-#include <memory>
 #include <Arduino.h>
 #include <StreamString.h>
 #include <WiFiClient.h>
 
+#include <memory>
+
 #ifdef DEBUG_ESP_HTTP_CLIENT
 #ifdef DEBUG_ESP_PORT
 #define DEBUG_HTTPCLIENT(fmt, ...) DEBUG_ESP_PORT.printf_P( (PGM_P)PSTR(fmt), ## __VA_ARGS__ )
@@ -151,8 +152,10 @@ typedef std::unique_ptr<TransportTraits> TransportTraitsPtr;
 class HTTPClient
 {
 public:
-    HTTPClient();
-    ~HTTPClient();
+    HTTPClient() = default;
+    ~HTTPClient() = default;
+    HTTPClient(HTTPClient&&) = default;
+    HTTPClient& operator=(HTTPClient&&) = default;
 
 /*
  * Since both begin() functions take a reference to client as a parameter, you need to 
@@ -223,6 +226,64 @@ class HTTPClient
         String value;
     };
 
+    // TODO: the common pattern to use the class is to
+    // {
+    //     WiFiClient socket;
+    //     HTTPClient http;
+    //     http.begin(socket, "http://blahblah");
+    // }
+    // in case wificlient supports seamless ref() / unref() of the underlying connection
+    // for both wificlient and wificlientsecure, this may be removed in favour of that approach.
+
+    struct NonOwningClientPtr {
+        NonOwningClientPtr() = default;
+        NonOwningClientPtr(const NonOwningClientPtr&) = delete;
+        NonOwningClientPtr(NonOwningClientPtr&& other) noexcept {
+            *this = std::move(other);
+        }
+
+        ~NonOwningClientPtr() noexcept {
+            if (_ptr) {
+                _ptr->stop();
+            }
+        }
+
+        explicit NonOwningClientPtr(WiFiClient* ptr) noexcept :
+            _ptr(ptr)
+        {}
+
+        explicit operator bool() const {
+            return _ptr != nullptr;
+        }
+
+        WiFiClient* get() const noexcept {
+            return _ptr;
+        }
+
+        WiFiClient& operator*() const {
+            return *_ptr;
+        }
+
+        WiFiClient* operator->() const noexcept {
+            return _ptr;
+        }
+
+        NonOwningClientPtr& operator=(WiFiClient* ptr) noexcept {
+            _ptr = ptr;
+            return *this;
+        }
+
+        NonOwningClientPtr& operator=(const NonOwningClientPtr&) = delete;
+        NonOwningClientPtr& operator=(NonOwningClientPtr&& other) noexcept {
+            _ptr = other._ptr;
+            other._ptr = nullptr;
+            return *this;
+        }
+
+    private:
+        WiFiClient* _ptr = nullptr;
+    };
+
     bool beginInternal(const String& url, const char* expectedProtocol);
     void disconnect(bool preserveClient = false);
     void clear();
@@ -232,7 +293,7 @@ class HTTPClient
     int handleHeaderResponse();
     int writeToStreamDataBlock(Stream * stream, int len);
 
-    WiFiClient* _client;
+    NonOwningClientPtr _client;
 
     /// request handling
     String _host;
@@ -244,12 +305,14 @@ class HTTPClient
     String _uri;
     String _protocol;
     String _headers;
-    String _userAgent;
     String _base64Authorization;
 
+    static const String defaultUserAgent;
+    String _userAgent = defaultUserAgent;
+
     /// Response handling
-    RequestArgument* _currentHeaders = nullptr;
-    size_t           _headerKeysCount = 0;
+    std::unique_ptr<RequestArgument[]> _currentHeaders;
+    size_t _headerKeysCount = 0;
 
     int _returnCode = 0;
     int _size = -1;
@@ -261,6 +324,4 @@ class HTTPClient
     std::unique_ptr<StreamString> _payload;
 };
 
-
-
 #endif /* ESP8266HTTPClient_H_ */