Adding option to http_client_config to specify whether or not the request body should be buffered.

This is useful for situations where authentication challenges might occur or autologon is used.
This is to help an external customer and is only avaliable on Windows desktop right now.

Author: stgates

Release/include/cpprest/http_client.h

@@ -96,7 +96,12 @@ class http_client_config
 #if !defined(__cplusplus_winrt)
         , m_validate_certificates(true)
 #endif
-        ,m_set_user_nativehandle_options([](native_handle)->void{})
+        , m_set_user_nativehandle_options([](native_handle)->void{})
+#ifdef _MS_WINDOWS
+#if !defined(__cplusplus_winrt)
+        , m_buffer_request(false)
+#endif
+#endif
     {
     }
 
@@ -212,6 +217,30 @@ class http_client_config
     }
 #endif
 
+#ifdef _MS_WINDOWS
+#if !defined(__cplusplus_winrt)
+    /// <summary>
+    /// Checks if request data buffering is turned on, the default is off.
+    /// </summary>
+    /// <returns>True if buffering is enabled, false otherwise</returns>
+    bool buffer_request() const
+    {
+        return m_buffer_request;
+    }
+
+    /// <summary>
+    /// Sets the request buffering property. 
+    /// If true, in cases where the request body/stream doesn't support seeking the request data will be buffered.
+    /// This can help in situations where an authentication challenge might be expected.
+    /// </summary>
+    /// <remarks>Please note there is a performance cost due to copying the request data.</remarks>
+    void set_buffer_request(bool buffer_request)
+    {
+        m_buffer_request = buffer_request;
+    }
+#endif
+#endif
+    
     /// <summary>
     /// Sets a callback to enable custom setting of winhttp options
     /// </summary>
@@ -231,6 +260,13 @@ class http_client_config
 #if !defined(__cplusplus_winrt)
     bool m_validate_certificates;
 #endif
+
+#ifdef _MS_WINDOWS
+#if !defined(__cplusplus_winrt)
+    bool m_buffer_request;
+#endif
+#endif
+
     std::function<void(native_handle)> m_set_user_nativehandle_options;
 
     utility::seconds m_timeout;
@@ -244,7 +280,6 @@ class http_client_config
 #endif // __cplusplus_winrt  
 #endif // _MS_WINDOWS
 
-
     /// <summary>
     /// Invokes a user callback to allow for customization of the requst
     /// </summary>

Release/include/cpprest/http_client_impl.h

@@ -187,7 +187,7 @@ class request_context
         finish();
     }
 
-    concurrency::streams::streambuf<uint8_t> _get_readbuffer()
+    virtual concurrency::streams::streambuf<uint8_t> _get_readbuffer()
     {
         auto instream = m_request.body();
 
@@ -200,7 +200,6 @@ class request_context
         auto outstream = m_response._get_impl()->outstream();
 
         _ASSERTE((bool)outstream);
-
         return outstream.streambuf();
     }
 
@@ -256,7 +255,7 @@ class _http_client_communicator
     // Destructor to clean up any held resources.
     virtual ~_http_client_communicator() {}
 
-    // Asychronously send a HTTP request and process the response.
+    // Asynchronously send a HTTP request and process the response.
     void async_send_request(std::shared_ptr<request_context> request)
     {
         if(m_client_config.guarantee_order())
@@ -388,7 +387,7 @@ class _http_client_communicator
         }
     }
 
-    // Queue used to guarantee ordering of requests, when appliable.
+    // Queue used to guarantee ordering of requests, when applicable.
     std::queue<std::shared_ptr<request_context>> m_requests_queue;
     int                           m_scheduled;
 };
@@ -424,7 +423,7 @@ class http_network_handler : public http_pipeline_stage
 // Helper function to check to make sure the uri is valid.
 void verify_uri(const uri &uri)
 {
-    // Somethings like proper URI schema are verified by the URI class.
+    // Some things like proper URI schema are verified by the URI class.
     // We only need to check certain things specific to HTTP.
     if (uri.scheme() != _XPLATSTR("http") && uri.scheme() != _XPLATSTR("https"))
     {

Release/src/http/client/http_win7.cpp

@@ -226,9 +226,14 @@ class winhttp_request_context : public request_context
 
     std::char_traits<uint8_t>::pos_type m_readbuf_pos;
 
-    // The read-write buffer used to save the outgoing body data,
-    // if the authentication challenges failed, resend the body data from this buffer.
-    std::shared_ptr<streams::producer_consumer_buffer<uint8_t>> m_rwbuf_copy;
+    // If the user specified that to guarantee data buffering of request data, in case of challenged authentication requests, etc...
+    // Then if the request stream buffer doesn't support seeking we need to copy the body chunks as it is sent.
+    concurrency::streams::istream m_readStream;
+    std::unique_ptr<concurrency::streams::container_buffer<std::vector<uint8_t>>> m_readBufferCopy;
+    virtual concurrency::streams::streambuf<uint8_t> _get_readbuffer()
+    {
+        return m_readStream.streambuf();
+    }
 
     memory_holder m_body_data;
 
@@ -261,7 +266,8 @@ class winhttp_request_context : public request_context
         m_body_data(),
         m_remaining_to_write(0),
         m_proxy_authentication_tried(false),
-        m_server_authentication_tried(false)
+        m_server_authentication_tried(false),
+        m_readStream(request.body())
     {
     }
 };
@@ -614,10 +620,10 @@ class winhttp_client : public _http_client_communicator
             return;
         }
 
-        // Initialize the local producer_consumer buffer.
-        if (winhttp_context->m_bodyType == transfer_encoding_chunked && !winhttp_context->_get_readbuffer().can_seek() && has_credentials(winhttp_context))
+        // Only need to cache the request body if user specified and the request stream doesn't support seeking.
+        if (winhttp_context->m_bodyType != no_body && client_config().buffer_request() && !winhttp_context->_get_readbuffer().can_seek())
         {
-            winhttp_context->m_rwbuf_copy = std::make_shared<streams::producer_consumer_buffer<uint8_t>>();
+            winhttp_context->m_readBufferCopy = ::utility::details::make_unique<::concurrency::streams::container_buffer<std::vector<uint8_t>>>();
         }
 
         _start_request_send(winhttp_context, content_length);
@@ -661,7 +667,7 @@ class winhttp_client : public _http_client_communicator
             return;
         }
 
-        // Capure the current read position of the stream.
+        // Capture the current read position of the stream.
         auto rbuf = winhttp_context->_get_readbuffer();
         if ( !_check_streambuf(winhttp_context, rbuf, _XPLATSTR("Input stream is not open")) )
         {
@@ -708,19 +714,10 @@ class winhttp_client : public _http_client_communicator
             try
             {
                 bytes_read = op.get();
-                if (!p_request_context->_get_readbuffer().can_seek() && has_credentials(p_request_context)
-                    && !p_request_context->m_server_authentication_tried && !p_request_context->m_proxy_authentication_tried)
+                // If the read buffer for copying exists then write to it.
+                if (p_request_context->m_readBufferCopy)
                 {
-                    // If the user buffer is not seekable, the credentials are provided and the authentication challenges are not tried,
-                    // copy the user buffer to a local producer_consumer buffer for resending if authentication failed.
-                    try
-                    {
-                        p_request_context->m_rwbuf_copy->putn(&p_request_context->m_body_data.get()[http::details::chunked_encoding::data_offset], bytes_read).wait();
-                    }
-                    catch (...)
-                    {
-                        p_request_context->report_exception(std::current_exception());
-                    }
+                    p_request_context->m_readBufferCopy->putn(&p_request_context->m_body_data.get()[http::details::chunked_encoding::data_offset], bytes_read).wait();
                 }
             }
             catch (...)
@@ -737,18 +734,11 @@ class winhttp_client : public _http_client_communicator
             if (bytes_read == 0)
             {
                 p_request_context->m_bodyType = no_body;
-                if (!p_request_context->_get_readbuffer().can_seek() && has_credentials(p_request_context)
-                    && !p_request_context->m_server_authentication_tried && !p_request_context->m_proxy_authentication_tried)
+                if (p_request_context->m_readBufferCopy)
                 {
-                    try
-                    {
-                        p_request_context->m_rwbuf_copy->close(std::ios_base::out).wait();
-
-                    }
-                    catch (...)
-                    {
-                        p_request_context->report_exception(std::current_exception());
-                    }
+                    // Move the saved buffer into the read buffer.
+                    p_request_context->m_readStream = concurrency::streams::container_stream<std::vector<uint8_t>>::open_istream(std::move(p_request_context->m_readBufferCopy->collection()));
+                    p_request_context->m_readBufferCopy.reset();
                 }
             }
 
@@ -764,24 +754,11 @@ class winhttp_client : public _http_client_communicator
             }
         };
 
-        if ((!p_request_context->m_server_authentication_tried && !p_request_context->m_proxy_authentication_tried) || p_request_context->_get_readbuffer().can_seek())
-        {
-            if (!_check_streambuf(p_request_context, p_request_context->_get_readbuffer(), _XPLATSTR("Input stream is not open")))
-            {
-                return;
-            }
-
-            p_request_context->_get_readbuffer().getn(&p_request_context->m_body_data.get()[http::details::chunked_encoding::data_offset], chunk_size).then(after_read);
-        }
-        else
+        if (!_check_streambuf(p_request_context, p_request_context->_get_readbuffer(), _XPLATSTR("Input stream is not open")))
         {
-            if (!_check_streambuf(p_request_context, *p_request_context->m_rwbuf_copy, _XPLATSTR("Locally copied input stream is not open")))
-            {
-                return;
-            }
-            // Resending the request after authentication failed, we need to read the body data from the copied buffer.
-            p_request_context->m_rwbuf_copy->getn(&p_request_context->m_body_data.get()[http::details::chunked_encoding::data_offset], chunk_size).then(after_read);
+            return;
         }
+        p_request_context->_get_readbuffer().getn(&p_request_context->m_body_data.get()[http::details::chunked_encoding::data_offset], chunk_size).then(after_read);
     }
 
     static void _multiple_segment_write_data(_In_ winhttp_request_context * p_request_context)
@@ -1333,7 +1310,7 @@ class winhttp_client : public _http_client_communicator
 http_network_handler::http_network_handler(uri base_uri, http_client_config client_config) :
     m_http_client_impl(std::make_shared<details::winhttp_client>(std::move(base_uri), std::move(client_config)))
 {
-    }
+}
 
 pplx::task<http_response> http_network_handler::propagate(http_request request)
 {

Release/tests/Functional/http/client/authentication_tests.cpp

@@ -490,6 +490,9 @@ TEST_FIXTURE(uri_address, auth_producer_comsumer_buffer)
     msg.set_body(buf.create_istream());
 
     http_client_config config;
+    VERIFY_IS_FALSE(config.buffer_request());
+    config.set_buffer_request(true);
+    VERIFY_IS_TRUE(config.buffer_request());
     config.set_credentials(credentials(U("USERNAME"), U("PASSWORD")));
 
     http_client client(m_uri, config);
@@ -553,6 +556,7 @@ TEST_FIXTURE(uri_address, auth_producer_comsumer_buffer_fail)
     msg.set_body(buf.create_istream());
 
     http_client_config config;
+    config.set_buffer_request(true);
     config.set_credentials(credentials(U("USERNAME"), U("PASSWORD")));
 
     http_client client(m_uri, config);