Addressing a bug in Windows http_listener with issues around the inco…

…ming request URI properly being parse and handled if errors occur.
bhassani · May 27, 2015 · 3454b26 · 3454b26
1 parent e26d019
commit 3454b26
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 15 deletions.
diff --git a/Release/src/http/listener/http_server_httpsys.cpp b/Release/src/http/listener/http_server_httpsys.cpp
@@ -544,22 +544,20 @@ void windows_request_context::read_headers_io_completion(DWORD error_code, DWORD
     }
     else
     {
-        // Parse headers.
-        // CookedUrl.pFullUrl contains the canonicalized URL and it is not encoded
-        // However, Query strings are opaque to http.sys and are passed as-is => CookedUrl.pFullUrl
-        // contains an already encoded version of query string.
-        uri_builder builder(uri::encode_uri(m_request->CookedUrl.pFullUrl));
-        if (m_request->CookedUrl.QueryStringLength != 0)
-        {
-            builder.set_query(uri::decode(builder.query()));
-        }
+        std::string badRequestMsg;
         try
         {
-            m_msg.set_request_uri(builder.to_uri());
+            // HTTP_REQUEST::pRawUrl contains the raw URI that came across the wire.
+            // Use this instead since the CookedUrl is a mess of the URI components
+            // some encoded and some not.
+            m_msg.set_request_uri(utf8_to_utf16(m_request->pRawUrl));
         }
         catch(const uri_exception &e)
         {
-            m_msg.reply(status_codes::BadRequest, e.what());
+            // If an exception occurred, finish processing the request below but
+            // respond with BadRequest instead of dispatching to the user's
+            // request handlers.
+            badRequestMsg = e.what();
         }
         m_msg.set_method(parse_request_method(m_request));
         parse_http_headers(m_request->Headers, m_msg.headers());
@@ -569,7 +567,14 @@ void windows_request_context::read_headers_io_completion(DWORD error_code, DWORD
         read_request_body_chunk();
 
         // Dispatch request to the http_listener.
-        dispatch_request_to_listener((web::http::experimental::listener::details::http_listener_impl *)m_request->UrlContext);
+        if(badRequestMsg.empty())
+        {
+            dispatch_request_to_listener((web::http::experimental::listener::details::http_listener_impl *)m_request->UrlContext);
+        }
+        else
+        {
+            m_msg.reply(status_codes::BadRequest, badRequestMsg);
+        }
     }
 }
 

diff --git a/Release/tests/functional/http/listener/request_handler_tests.cpp b/Release/tests/functional/http/listener/request_handler_tests.cpp
@@ -181,9 +181,9 @@ TEST_FIXTURE(uri_address, handle_trace)
         std::string utf8_response;
         utf8_response.assign(p_response->m_data.begin(), p_response->m_data.end());
 #ifdef _WIN32
-        VERIFY_ARE_EQUAL("TRACE http://localhost:34567/ HTTP/1.1\r\nConnection: Keep-Alive\r\nHost: localhost:34567\r\nUser-Agent: test_http_client\r\n\r\n", utf8_response);
+        VERIFY_ARE_EQUAL("TRACE / HTTP/1.1\r\nConnection: Keep-Alive\r\nHost: localhost:34567\r\nUser-Agent: test_http_client\r\n\r\n", utf8_response);
 #else
-        VERIFY_ARE_EQUAL("TRACE http://localhost:34567/ HTTP/1.1\r\nConnection: Keep-Alive\r\nContent-Length: 0\r\nContent-Type: text/plain; charset=utf-8\r\nHost: localhost:34567\r\nUser-Agent: test_http_client\r\n\r\n", utf8_response);
+        VERIFY_ARE_EQUAL("TRACE / HTTP/1.1\r\nConnection: Keep-Alive\r\nContent-Length: 0\r\nContent-Type: text/plain; charset=utf-8\r\nHost: localhost:34567\r\nUser-Agent: test_http_client\r\n\r\n", utf8_response);
 #endif
     }).wait();
 

diff --git a/Release/tests/functional/http/listener/to_string_tests.cpp b/Release/tests/functional/http/listener/to_string_tests.cpp
@@ -62,7 +62,7 @@ TEST_FIXTURE(uri_address, request_to_string)
         http_asserts::assert_request_string_equals(
             request.to_string(),
             U("GET"),
-            U("http://localhost:34567/pa%20th1"),
+            U("/pa%20th1"),
             U("HTTP/1.1"),
             expected_headers,
             U("hehehe"));