⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 243252 in webkit


Ignore:
Timestamp:
Mar 20, 2019, 4:15:04 PM (7 years ago)
Author:
achristensen@apple.com
Message:

Use WeakPtr instead of storing raw pointers in WebSocket code
​https://bugs.webkit.org/show_bug.cgi?id=196034

Reviewed by Geoff Garen.

This could prevent using freed memory if we forget to reset a pointer somewhere.

  • Modules/websockets/WebSocketChannel.cpp:

(WebCore::WebSocketChannel::WebSocketChannel):
(WebCore::WebSocketChannel::connect):
(WebCore::WebSocketChannel::fail):
(WebCore::WebSocketChannel::disconnect):
(WebCore::WebSocketChannel::didOpenSocketStream):
(WebCore::WebSocketChannel::didCloseSocketStream):
(WebCore::WebSocketChannel::didFailSocketStream):
(WebCore::WebSocketChannel::processBuffer):
(WebCore::WebSocketChannel::processFrame):
(WebCore::WebSocketChannel::processOutgoingFrameQueue):
(WebCore::WebSocketChannel::sendFrame):

  • Modules/websockets/WebSocketChannel.h:
  • Modules/websockets/WebSocketChannelClient.h:
  • Modules/websockets/WebSocketHandshake.cpp:

(WebCore::WebSocketHandshake::WebSocketHandshake):

  • Modules/websockets/WebSocketHandshake.h:
Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r243249 r243252  
     12019-03-20  Alex Christensen  <achristensen@webkit.org>
     2
     3        Use WeakPtr instead of storing raw pointers in WebSocket code
     4        https://bugs.webkit.org/show_bug.cgi?id=196034
     5
     6        Reviewed by Geoff Garen.
     7
     8        This could prevent using freed memory if we forget to reset a pointer somewhere.
     9
     10        * Modules/websockets/WebSocketChannel.cpp:
     11        (WebCore::WebSocketChannel::WebSocketChannel):
     12        (WebCore::WebSocketChannel::connect):
     13        (WebCore::WebSocketChannel::fail):
     14        (WebCore::WebSocketChannel::disconnect):
     15        (WebCore::WebSocketChannel::didOpenSocketStream):
     16        (WebCore::WebSocketChannel::didCloseSocketStream):
     17        (WebCore::WebSocketChannel::didFailSocketStream):
     18        (WebCore::WebSocketChannel::processBuffer):
     19        (WebCore::WebSocketChannel::processFrame):
     20        (WebCore::WebSocketChannel::processOutgoingFrameQueue):
     21        (WebCore::WebSocketChannel::sendFrame):
     22        * Modules/websockets/WebSocketChannel.h:
     23        * Modules/websockets/WebSocketChannelClient.h:
     24        * Modules/websockets/WebSocketHandshake.cpp:
     25        (WebCore::WebSocketHandshake::WebSocketHandshake):
     26        * Modules/websockets/WebSocketHandshake.h:
     27
    1282019-03-20  Dean Jackson  <dino@apple.com>
    229

  • trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp

    r241244 r243252  
    6464
    6565WebSocketChannel::WebSocketChannel(Document& document, WebSocketChannelClient& client, SocketProvider& provider)
    66     : m_document(&document)
    67     , m_client(&client)
     66    : m_document(makeWeakPtr(document))
     67    , m_client(makeWeakPtr(client))
    6868    , m_resumeTimer(*this, &WebSocketChannel::resumeTimerFired)
    6969    , m_closingTimer(*this, &WebSocketChannel::closingTimerFired)
    … …  
    113113    ASSERT(!m_handle);
    114114    ASSERT(!m_suspended);
    115     m_handshake = std::make_unique<WebSocketHandshake>(url, protocol, m_document, allowCookies);
     115    m_handshake = std::make_unique<WebSocketHandshake>(url, protocol, m_document.get(), allowCookies);
    116116    m_handshake->reset();
    117117    if (m_deflateFramer.canDeflate())
    118118        m_handshake->addExtensionProcessor(m_deflateFramer.createExtensionProcessor());
    119119    if (m_identifier)
    120         InspectorInstrumentation::didCreateWebSocket(m_document, m_identifier, url);
     120        InspectorInstrumentation::didCreateWebSocket(m_document.get(), m_identifier, url);
    121121
    122122    if (Frame* frame = m_document->frame()) {
    … …  
    215215    ASSERT(!m_suspended);
    216216    if (m_document) {
    217         InspectorInstrumentation::didReceiveWebSocketFrameError(m_document, m_identifier, reason);
     217        InspectorInstrumentation::didReceiveWebSocketFrameError(m_document.get(), m_identifier, reason);
    218218
    219219        String consoleMessage;
    … …  
    246246    LOG(Network, "WebSocketChannel %p disconnect()", this);
    247247    if (m_identifier && m_document)
    248         InspectorInstrumentation::didCloseWebSocket(m_document, m_identifier);
     248        InspectorInstrumentation::didCloseWebSocket(m_document.get(), m_identifier);
    249249    if (m_handshake)
    250250        m_handshake->clearDocument();
    … …  
    274274        return;
    275275    if (m_identifier && UNLIKELY(InspectorInstrumentation::hasFrontends()))
    276         InspectorInstrumentation::willSendWebSocketHandshakeRequest(m_document, m_identifier, m_handshake->clientHandshakeRequest());
     276        InspectorInstrumentation::willSendWebSocketHandshakeRequest(m_document.get(), m_identifier, m_handshake->clientHandshakeRequest());
    277277    auto handshakeMessage = m_handshake->clientHandshakeMessage();
    278278    auto cookieRequestHeaderFieldProxy = m_handshake->clientHandshakeCookieRequestHeaderFieldProxy();
    … …  
    290290    LOG(Network, "WebSocketChannel %p didCloseSocketStream()", this);
    291291    if (m_identifier && m_document)
    292         InspectorInstrumentation::didCloseWebSocket(m_document, m_identifier);
     292        InspectorInstrumentation::didCloseWebSocket(m_document.get(), m_identifier);
    293293    ASSERT_UNUSED(handle, &handle == m_handle || !m_handle);
    294294    m_closed = true;
    … …  
    301301        if (m_suspended)
    302302            return;
    303         WebSocketChannelClient* client = m_client;
     303        WebSocketChannelClient* client = m_client.get();
    304304        m_client = nullptr;
    305305        m_document = nullptr;
    … …  
    364364        else
    365365            message = "WebSocket network error: " + error.localizedDescription();
    366         InspectorInstrumentation::didReceiveWebSocketFrameError(m_document, m_identifier, message);
     366        InspectorInstrumentation::didReceiveWebSocketFrameError(m_document.get(), m_identifier, message);
    367367        m_document->addConsoleMessage(MessageSource::Network, MessageLevel::Error, message);
    368368    }
    … …  
    449449        if (m_handshake->mode() == WebSocketHandshake::Connected) {
    450450            if (m_identifier)
    451                 InspectorInstrumentation::didReceiveWebSocketHandshakeResponse(m_document, m_identifier, m_handshake->serverHandshakeResponse());
     451                InspectorInstrumentation::didReceiveWebSocketHandshakeResponse(m_document.get(), m_identifier, m_handshake->serverHandshakeResponse());
    452452            String serverSetCookie = m_handshake->serverSetCookie();
    453453            if (!serverSetCookie.isEmpty()) {
    … …  
    583583    }
    584584
    585     InspectorInstrumentation::didReceiveWebSocketFrame(m_document, m_identifier, frame);
     585    InspectorInstrumentation::didReceiveWebSocketFrame(m_document.get(), m_identifier, frame);
    586586
    587587    switch (frame.opCode) {
    … …  
    769769                m_blobLoader = std::make_unique<FileReaderLoader>(FileReaderLoader::ReadAsArrayBuffer, this);
    770770                m_blobLoaderStatus = BlobLoaderStarted;
    771                 m_blobLoader->start(m_document, *frame->blobData);
     771                m_blobLoader->start(m_document.get(), *frame->blobData);
    772772                m_outgoingFrameQueue.prepend(WTFMove(frame));
    773773                return;
    … …  
    821821
    822822    WebSocketFrame frame(opCode, true, false, true, data, dataLength);
    823     InspectorInstrumentation::didSendWebSocketFrame(m_document, m_identifier, frame);
     823    InspectorInstrumentation::didSendWebSocketFrame(m_document.get(), m_identifier, frame);
    824824
    825825    auto deflateResult = m_deflateFramer.deflate(frame);

  • trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h

    r241824 r243252  
    194194    };
    195195
    196     Document* m_document;
    197     WebSocketChannelClient* m_client;
     196    WeakPtr<Document> m_document;
     197    WeakPtr<WebSocketChannelClient> m_client;
    198198    std::unique_ptr<WebSocketHandshake> m_handshake;
    199199    RefPtr<SocketStreamHandle> m_handle;

  • trunk/Source/WebCore/Modules/websockets/WebSocketChannelClient.h

    r223728 r243252  
    3232
    3333#include <wtf/Forward.h>
     34#include <wtf/WeakPtr.h>
    3435
    3536namespace WebCore {
    3637
    37 class WebSocketChannelClient {
     38class WebSocketChannelClient : public CanMakeWeakPtr<WebSocketChannelClient> {
    3839public:
    3940    virtual ~WebSocketChannelClient() = default;

  • trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp

    r241244 r243252  
    124124    , m_clientProtocol(protocol)
    125125    , m_secure(m_url.protocolIs("wss"))
    126     , m_document(document)
     126    , m_document(makeWeakPtr(document))
    127127    , m_mode(Incomplete)
    128128    , m_allowCookies(allowCookies)

  • trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.h

    r239427 r243252  
    3636#include "WebSocketExtensionDispatcher.h"
    3737#include "WebSocketExtensionProcessor.h"
     38#include <wtf/WeakPtr.h>
    3839#include <wtf/text/WTFString.h>
    3940
    … …  
    101102    String m_clientProtocol;
    102103    bool m_secure;
    103     Document* m_document;
     104    WeakPtr<Document> m_document;
    104105
    105106    Mode m_mode;
Note: See TracChangeset for help on using the changeset viewer.