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,31 @@ 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>
+    /// <param name="buffer_request">True to turn on buffer, false otherwise.</param>
+    /// <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 +261,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 +281,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

@@ -224,11 +224,16 @@ class winhttp_request_context : public request_context
 
     size64_t m_remaining_to_write;
 
-    std::char_traits<uint8_t>::pos_type m_readbuf_pos;
+    std::char_traits<uint8_t>::pos_type m_startingPosition;
 
-    // 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;
 
@@ -257,11 +262,12 @@ class winhttp_request_context : public request_context
         : request_context(client, request), 
         m_request_handle(nullptr), 
         m_bodyType(no_body),
-        m_readbuf_pos(0),
+        m_startingPosition(std::char_traits<uint8_t>::eof()),
         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,14 +667,16 @@ 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")) )
         {
             return;
         }
 
-        winhttp_context->m_readbuf_pos = rbuf.getpos(std::ios_base::in);
+        // Record starting position incase request is challenged for authorization
+        // and needs to seek back to where reading is started from.
+        winhttp_context->m_startingPosition = rbuf.getpos(std::ios_base::in);
 
         // If we find ourselves here, we either don't know how large the message
         // body is, or it is larger than our threshold.
@@ -708,19 +716,12 @@ 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());
-                    }
+                    // We have raw memory here writing to a memory stream so it is safe to wait
+                    // since it will always be non-blocking.
+                    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 +738,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, which now supports seeking.
+                    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 +758,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)
@@ -887,10 +868,6 @@ class winhttp_client : public _http_client_communicator
         _ASSERTE(response.status_code() == status_codes::Unauthorized  || response.status_code() == status_codes::ProxyAuthRequired
             || error == ERROR_WINHTTP_RESEND_REQUEST);
 
-        // If the application set a stream for the request body, we can only resend if the input stream supports
-        // seeking and we are also successful in seeking to the position we started at when the original request
-        // was sent.
-
         bool got_credentials = false;
         BOOL results;
         DWORD dwSupportedSchemes;
@@ -900,22 +877,16 @@ class winhttp_client : public _http_client_communicator
         string_t username;
         string_t password;
 
-        if (request.body())
+        // Check if the saved read position is valid
+        auto rdpos = p_request_context->m_startingPosition;
+        if (rdpos != static_cast<std::char_traits<uint8_t>::pos_type>(std::char_traits<uint8_t>::eof()))
         {
-            // Valid request stream => msg has a body that needs to be resend
+            auto rbuf = p_request_context->_get_readbuffer();
 
-            auto rdpos = p_request_context->m_readbuf_pos;
-
-            // Check if the saved read position is valid
-            if (rdpos != (std::char_traits<uint8_t>::pos_type) - 1)
+            // Try to seek back to the saved read position
+            if (rbuf.seekpos(rdpos, std::ios::ios_base::in) != rdpos)
             {
-                auto rbuf = p_request_context->_get_readbuffer();
-
-                if (rbuf.is_open())
-                {
-                    // Try to seek back to the saved read position
-                    rbuf.seekpos(rdpos, std::ios::ios_base::in);
-                }
+                return false;
             }
         }
 
@@ -1333,7 +1304,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);