Skip to content

Commit

Permalink
SSLClientSocket::IsConnected should care for internal buffers
Browse files Browse the repository at this point in the history
SSLClientSocket::IsConnected() and SSLClientSocket::IsConnectedAndIdle()
may return false though it has buffered data. They should care for internally
processing buffer.

There are various implementation, for NSS, OpenSSL, and platform dependent
system libraries. This CL fix the issue on NSS. Fix for others will follow.

BUG=160033
TEST=browser_tests, net_unittests

Review URL: https://codereview.chromium.org/11366155

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@178775 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
toyoshim@chromium.org committed Jan 25, 2013
1 parent b03766f commit 32cbd36
Show file tree
Hide file tree
Showing 8 changed files with 324 additions and 29 deletions.
94 changes: 94 additions & 0 deletions chrome/browser/net/websocket_browsertest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/string_util.h"
#include "base/utf_string_conversions.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_tabstrip.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/test/browser_test_utils.h"
#include "net/base/test_data_directory.h"
#include "net/test/test_server.h"

namespace {

class WebSocketBrowserTest : public InProcessBrowserTest {
public:
WebSocketBrowserTest()
: ws_server_(net::TestServer::TYPE_WS,
net::TestServer::kLocalhost,
net::GetWebSocketTestDataDirectory()),
wss_server_(net::TestServer::TYPE_WSS,
SSLOptions(SSLOptions::CERT_OK),
net::GetWebSocketTestDataDirectory()) {
}

protected:
net::TestServer ws_server_;
net::TestServer wss_server_;

private:
typedef net::TestServer::SSLOptions SSLOptions;

DISALLOW_COPY_AND_ASSIGN(WebSocketBrowserTest);
};

// Test that the browser can handle a WebSocket frame split into multiple TCP
// segments.
IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, WebSocketSplitSegments) {
// Launch a WebSocket server.
ASSERT_TRUE(ws_server_.Start());

// Setup page title observer.
content::WebContents* tab = chrome::GetActiveWebContents(browser());
content::TitleWatcher watcher(tab, ASCIIToUTF16("PASS"));
watcher.AlsoWaitForTitle(ASCIIToUTF16("FAIL"));

// Visit a HTTP page for testing.
std::string scheme("http");
GURL::Replacements replacements;
replacements.SetSchemeStr(scheme);
ui_test_utils::NavigateToURL(
browser(),
ws_server_.GetURL(
"split_packet_check.html").ReplaceComponents(replacements));

const string16 result = watcher.WaitAndGetTitle();
EXPECT_TRUE(EqualsASCII(result, "PASS"));
}

#if defined(USE_OPENSSL)
// TODO(toyoshim): Enable this test for OpenSSL once http://crbug.com/160033
// is fixed against OpenSSL.
#define MAYBE_SecureWebSocketSplitRecords DISABLED_SecureWebSocketSplitRecords
#else
#define MAYBE_SecureWebSocketSplitRecords SecureWebSocketSplitRecords
#endif
// Test that the browser can handle a WebSocket frame split into multiple SSL
// records.
IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest,
MAYBE_SecureWebSocketSplitRecords) {
// Launch a secure WebSocket server.
ASSERT_TRUE(wss_server_.Start());

// Setup page title observer.
content::WebContents* tab = chrome::GetActiveWebContents(browser());
content::TitleWatcher watcher(tab, ASCIIToUTF16("PASS"));
watcher.AlsoWaitForTitle(ASCIIToUTF16("FAIL"));

// Visit a HTTPS page for testing.
std::string scheme("https");
GURL::Replacements replacements;
replacements.SetSchemeStr(scheme);
ui_test_utils::NavigateToURL(
browser(),
wss_server_.GetURL(
"split_packet_check.html").ReplaceComponents(replacements));

const string16 result = watcher.WaitAndGetTitle();
EXPECT_TRUE(EqualsASCII(result, "PASS"));
}

} // namespace
1 change: 1 addition & 0 deletions chrome/chrome_tests.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -1182,6 +1182,7 @@
'browser/net/load_timing_observer_browsertest.cc',
'browser/net/predictor_browsertest.cc',
'browser/net/proxy_browsertest.cc',
'browser/net/websocket_browsertest.cc',
'browser/page_cycler/page_cycler_browsertest.cc',
'browser/performance_monitor/performance_monitor_browsertest.cc',
'browser/policy/cloud_policy_browsertest.cc',
Expand Down
8 changes: 8 additions & 0 deletions net/base/nss_memio.c
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,14 @@ int memio_GetReadParams(memio_Private *secret, char **buf)
return memio_buffer_unused_contiguous(mb);
}

int memio_GetReadableBufferSize(memio_Private *secret)
{
struct memio_buffer* mb = &((PRFilePrivate *)secret)->readbuf;
PR_ASSERT(mb->bufsize);

return memio_buffer_used_contiguous(mb);
}

void memio_PutReadResult(memio_Private *secret, int bytes_read)
{
struct memio_buffer* mb = &((PRFilePrivate *)secret)->readbuf;
Expand Down
5 changes: 5 additions & 0 deletions net/base/nss_memio.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ int memio_GetReadRequest(memio_Private *secret);
*/
int memio_GetReadParams(memio_Private *secret, char **buf);

/* Ask memio how many bytes are contained in the internal buffer.
* Returns bytes available to read, or 0 if none available.
*/
int memio_GetReadableBufferSize(memio_Private *secret);

/* Tell memio how many bytes were read from the network.
* If bytes_read is 0, causes EOF to be reported to
* NSS after it reads the last byte from the circular buffer.
Expand Down
13 changes: 13 additions & 0 deletions net/data/websocket/README
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ Multiple tests may share one resource, or URI handler.
SSLUITest.TestWSSInvalidCertAndGoForward, SSLUITest.TestWSSClientCert,
and SSLUITestIgnoreCertErrors.TestWSS.

- split_packet_check.html : A page for testing split packet handling. Here,
packets mean TCP segments for WebSocket, or SSL records for secure
WebSocket. This page changes the title to "PASS" to notify
content::TitleWatcher.
Used by WebSocketBrowserTest.WebSocketSplitSegments and
WebSocketBrowserTest.SecureWebSocketSplitRecords.

- websocket_shared_worker.html : A page provides simple WebSocket test in
shared worker. This page changes page title to "PASS" to notify
content::TitleWatcher.
Expand All @@ -31,6 +38,12 @@ Multiple tests may share one resource, or URI handler.
connection with arbitrary code and reason.
Used by kinds of PPAPI tests for WebSocket.

- close-with-split-packet_wsh.py : A WebSocket URL handler for testing split
packet handling. Here, packets mean TCP segments for WebSocket, or SSL
records for secure WebSocket.
Used by WebSocketBrowserTest.WebSocketSplitSegments and
WebSocketBrowserTest.SecureWebSocketSplitRecords.

- protocol-test_wsh.py : A WebSocket URL handler for testing outgoing opening
handshake protocol.
Used by kinds of PPAPI tests for WebSocket.
30 changes: 30 additions & 0 deletions net/data/websocket/close-with-split-packet_wsh.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Copyright (c) 2012 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import struct

from mod_pywebsocket import handshake
from mod_pywebsocket import stream


def web_socket_do_extra_handshake(_request):
pass


def web_socket_transfer_data(request):
line = request.ws_stream.receive_message()
if line is None:
return
if isinstance(line, unicode):
request.ws_stream.send_message(line, binary=False)
else:
request.ws_stream.send_message(line, binary=True)


def web_socket_passive_closing_handshake(request):
code = struct.pack('!H', 1000)
packet = stream.create_close_frame(code + 'split test'.encode('utf-8'))
request.connection.write(packet[:1])
request.connection.write(packet[1:])
raise handshake.AbortedByUserException('Abort the connection')
37 changes: 37 additions & 0 deletions net/data/websocket/split_packet_check.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<!DOCTYPE html>
<html>
<head>
<title>test ws split packet</title>
<script type="text/javascript">

var href = window.location.href;
var hostBegin = href.indexOf('/') + 2;
var hostEnd = href.lastIndexOf(':');
var host = href.slice(hostBegin, hostEnd);
var portBegin = hostEnd + 1;
var portEnd = href.lastIndexOf('/');
var port = href.slice(portBegin, portEnd);
var scheme = href.indexOf('https') >= 0 ? 'wss' : 'ws';
var url = scheme + '://' + host + ':' + port + '/close-with-split-packet';

// Do connection test.
var ws = new WebSocket(url);

ws.onopen = function()
{
// Close WebSocket connection once it is established.
ws.close();
}

ws.onclose = function(event)
{
// Check wasClean, then set proper title.
if (event.wasClean)
document.title = 'PASS';
else
document.title = 'FAIL';
}

</script>
</head>
</html>
Loading

0 comments on commit 32cbd36

Please sign in to comment.