Changeset 262018 in webkit


Ignore:
Timestamp:
May 21, 2020, 11:52:06 AM (5 years ago)
Author:
achristensen@apple.com
Message:

[macOS] TestWebKitAPI.WebKit.HTTPReferer is a flaky failure
​https://bugs.webkit.org/show_bug.cgi?id=212180

Reviewed by Darin Adler.

Sometimes an HTTP request takes more than one call to nw_connection_receive to receive entirely.
Add a new abstraction Connection that wraps an nw_connection_t and knows how to read an entire request.
Use strnstr instead of null terminating and using strstr.

  • TestWebKitAPI/Tests/WebKitCocoa/Download.mm:

(TEST):

  • TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:

(TEST):

  • TestWebKitAPI/Tests/WebKitCocoa/Proxy.mm:

(TestWebKitAPI::TEST):

  • TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
  • TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerTCPServer.h:
  • TestWebKitAPI/cocoa/HTTPServer.h:

(TestWebKitAPI::Connection::receiveHTTPRequest):
(TestWebKitAPI::Connection::Connection):

  • TestWebKitAPI/cocoa/HTTPServer.mm:

(TestWebKitAPI::HTTPServer::HTTPServer):
(TestWebKitAPI::dataFromString):
(TestWebKitAPI::vectorFromData):
(TestWebKitAPI::HTTPServer::respondToRequests):
(TestWebKitAPI::HTTPServer::request const):
(TestWebKitAPI::Connection::receiveHTTPRequest const):
(TestWebKitAPI::Connection::send const):
(TestWebKitAPI::Connection::terminate const):
(TestWebKitAPI::nullTerminatedRequest): Deleted.

Location:
trunk/Tools
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r261986 r262018  
     12020-05-21  Alex Christensen  <achristensen@webkit.org>
     2
     3        [macOS] TestWebKitAPI.WebKit.HTTPReferer is a flaky failure
     4        https://bugs.webkit.org/show_bug.cgi?id=212180
     5
     6        Reviewed by Darin Adler.
     7
     8        Sometimes an HTTP request takes more than one call to nw_connection_receive to receive entirely.
     9        Add a new abstraction Connection that wraps an nw_connection_t and knows how to read an entire request.
     10        Use strnstr instead of null terminating and using strstr.
     11
     12        * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
     13        (TEST):
     14        * TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:
     15        (TEST):
     16        * TestWebKitAPI/Tests/WebKitCocoa/Proxy.mm:
     17        (TestWebKitAPI::TEST):
     18        * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
     19        * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerTCPServer.h:
     20        * TestWebKitAPI/cocoa/HTTPServer.h:
     21        (TestWebKitAPI::Connection::receiveHTTPRequest):
     22        (TestWebKitAPI::Connection::Connection):
     23        * TestWebKitAPI/cocoa/HTTPServer.mm:
     24        (TestWebKitAPI::HTTPServer::HTTPServer):
     25        (TestWebKitAPI::dataFromString):
     26        (TestWebKitAPI::vectorFromData):
     27        (TestWebKitAPI::HTTPServer::respondToRequests):
     28        (TestWebKitAPI::HTTPServer::request const):
     29        (TestWebKitAPI::Connection::receiveHTTPRequest const):
     30        (TestWebKitAPI::Connection::send const):
     31        (TestWebKitAPI::Connection::terminate const):
     32        (TestWebKitAPI::nullTerminatedRequest): Deleted.
     33
    1342020-05-21  Enrique Ocaña González  <eocanha@igalia.com>
    235

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm

    r261898 r262018  
    11731173{
    11741174    using namespace TestWebKitAPI;
    1175     HTTPServer server([connectionCount = 0](nw_connection_t connection) mutable {
     1175    HTTPServer server([connectionCount = 0](Connection connection) mutable {
    11761176        switch (++connectionCount) {
    11771177        case 1:
    1178             nw_connection_receive(connection, 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([connection = retainPtr(connection)](dispatch_data_t content, nw_content_context_t context, bool, nw_error_t error) {
    1179                 EXPECT_TRUE(content);
    1180                 EXPECT_FALSE(error);
    1181                 auto data = dataFromString(makeString(
     1178            connection.receiveHTTPRequest([connection](Vector<char>&&) {
     1179                connection.send(makeString(
    11821180                    "HTTP/1.1 200 OK\r\n"
    11831181                    "ETag: test\r\n"
    11841182                    "Content-Length: 10000\r\n"
    11851183                    "Content-Disposition: attachment; filename=\"example.txt\"\r\n"
    1186                     "\r\n", longString<5000>('a')));
    1187                 nw_connection_send(connection.get(), data.get(), context, false, ^(nw_error_t error) {
    1188                     ASSERT(!error);
    1189                 });
    1190             }).get());
     1184                    "\r\n", longString<5000>('a')
     1185                ));
     1186            });
    11911187            break;
    11921188        case 2:
    1193             nw_connection_receive(connection, 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([connection = retainPtr(connection)](dispatch_data_t content, nw_content_context_t context, bool, nw_error_t error) {
    1194                 EXPECT_TRUE(content);
    1195                 EXPECT_FALSE(error);
    1196 
    1197                 auto request = nullTerminatedRequest(content);
    1198                 EXPECT_TRUE(strstr(request.data(), "Range: bytes=5000-\r\n"));
    1199 
    1200                 auto data = dataFromString(makeString(
     1189            connection.receiveHTTPRequest([connection](Vector<char>&& request) {
     1190                EXPECT_TRUE(strnstr(request.data(), "Range: bytes=5000-\r\n", request.size()));
     1191                connection.send(makeString(
    12011192                    "HTTP/1.1 206 Partial Content\r\n"
    12021193                    "ETag: test\r\n"
    12031194                    "Content-Length: 5000\r\n"
    12041195                    "Content-Range: bytes 5000-9999/10000\r\n"
    1205                     "\r\n", longString<5000>('b')));
    1206                 nw_connection_send(connection.get(), data.get(), context, true, ^(nw_error_t error) {
    1207                     ASSERT(!error);
    1208                 });
    1209             }).get());
     1196                    "\r\n", longString<5000>('b')
     1197                ));
     1198            });
    12101199            break;
    12111200        default:

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm

    r261414 r262018  
    5555        using namespace TestWebKitAPI;
    5656        bool done = false;
    57         HTTPServer server([done = &done, expectedReferer] (nw_connection_t connection) {
    58             nw_connection_receive(connection, 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([connection = retainPtr(connection), done, expectedReferer](dispatch_data_t content, nw_content_context_t, bool, nw_error_t) {
    59                 EXPECT_TRUE(content);
    60                 auto request = nullTerminatedRequest(content);
     57        HTTPServer server([&] (Connection connection) {
     58            connection.receiveHTTPRequest([connection, expectedReferer, &done] (Vector<char>&& request) {
    6159                if (expectedReferer) {
    6260                    auto expectedHeaderField = makeString("Referer: ", expectedReferer, "\r\n");
    63                     EXPECT_TRUE(strstr(request.data(), expectedHeaderField.utf8().data()));
     61                    EXPECT_TRUE(strnstr(request.data(), expectedHeaderField.utf8().data(), request.size()));
    6462                } else
    65                     EXPECT_FALSE(strstr(request.data(), "Referer:"));
    66                 *done = true;
    67             }).get());
     63                    EXPECT_FALSE(strnstr(request.data(), "Referer:", request.size()));
     64                done = true;
     65            });
    6866        });
    6967        auto webView = adoptNS([WKWebView new]);

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Proxy.mm

    r259910 r262018  
    2828#if HAVE(SSL)
    2929
     30#import "HTTPServer.h"
    3031#import "PlatformUtilities.h"
    3132#import "TCPServer.h"
     33#import "TestNavigationDelegate.h"
     34#import "TestUIDelegate.h"
     35#import "TestWKWebView.h"
    3236#import "Utilities.h"
    3337#import <WebKit/WKWebsiteDataStorePrivate.h>
    … …  
    3539#import <WebKit/_WKWebsiteDataStoreConfiguration.h>
    3640#import <wtf/RetainPtr.h>
     41#import <wtf/text/StringConcatenateNumbers.h>
    3742
    3843@interface ProxyDelegate : NSObject <WKNavigationDelegate, WKUIDelegate>
    … …  
    9297    TCPServer server([] (int socket) {
    9398        auto requestShouldContain = [] (const auto& request, const char* str) {
    94             EXPECT_TRUE(strstr(reinterpret_cast<const char*>(request.data()), str));
     99            EXPECT_TRUE(strnstr(reinterpret_cast<const char*>(request.data()), str, request.size()));
    95100        };
    96101

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm

    r259540 r262018  
    20712071        respond(contentRuleListWorkerScript, "application/javascript");
    20722072        auto lastRequest = TCPServer::read(socket);
    2073         EXPECT_TRUE(strstr((const char*)lastRequest.data(), "allowedsubresource"));
     2073        EXPECT_TRUE(strnstr((const char*)lastRequest.data(), "allowedsubresource", lastRequest.size()));
    20742074        respond("successful fetch", "application/octet-stream");
    20752075    });

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerTCPServer.h

    r251384 r262018  
    4646                    auto request = TCPServer::read(socket);
    4747                    if (!expectedUserAgent.isNull()) {
    48                         EXPECT_TRUE(strstr((const char*)request.data(), makeString("User-Agent: ", expectedUserAgent).utf8().data()));
     48                        EXPECT_TRUE(strnstr((const char*)request.data(), makeString("User-Agent: ", expectedUserAgent).utf8().data(), request.size()));
    4949                        m_userAgentsChecked++;
    5050                    }

  • trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h

    r260518 r262018  
    3131
    3232#import <Network/Network.h>
     33#import <wtf/CompletionHandler.h>
    3334#import <wtf/Forward.h>
    3435#import <wtf/HashMap.h>
    … …  
    3637
    3738namespace TestWebKitAPI {
     39
     40class Connection;
    3841
    3942class HTTPServer {
    … …  
    4548
    4649    HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>>, Protocol = Protocol::Http, CertificateVerifier&& = nullptr);
    47     HTTPServer(Function<void(nw_connection_t)>&&, Protocol = Protocol::Http);
     50    HTTPServer(Function<void(Connection)>&&, Protocol = Protocol::Http);
    4851    ~HTTPServer();
    4952    uint16_t port() const;
    50     NSURLRequest *request() const;
     53    NSURLRequest *request(const String& path = "/"_str) const;
    5154    size_t totalRequests() const;
    5255
    5356private:
    5457    static RetainPtr<nw_parameters_t> listenerParameters(Protocol, CertificateVerifier&&);
    55     static void respondToRequests(nw_connection_t, RefPtr<RequestData>);
     58    static void respondToRequests(Connection, RefPtr<RequestData>);
    5659
    5760    RefPtr<RequestData> m_requestData;
    … …  
    6063};
    6164
    62 RetainPtr<dispatch_data_t> dataFromString(String&&);
    63 Vector<char> nullTerminatedRequest(dispatch_data_t);
     65class Connection {
     66public:
     67    void send(String&&, CompletionHandler<void()>&& = nullptr) const;
     68    void receiveHTTPRequest(CompletionHandler<void(Vector<char>&&)>&&, Vector<char>&& buffer = { }) const;
     69    void terminate() const;
     70
     71private:
     72    friend class HTTPServer;
     73    Connection(nw_connection_t connection)
     74        : m_connection(connection) { }
     75
     76    RetainPtr<nw_connection_t> m_connection;
     77};
    6478
    6579struct HTTPServer::HTTPResponse {

  • trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm

    r261101 r262018  
    9696        nw_connection_set_queue(connection, dispatch_get_main_queue());
    9797        nw_connection_start(connection);
    98         respondToRequests(connection, requestData);
     98        respondToRequests(Connection(connection), requestData);
    9999    }).get());
    100100    startListening(m_listener.get());
    101101}
    102102
    103 HTTPServer::HTTPServer(Function<void(nw_connection_t)>&& connectionHandler, Protocol protocol)
     103HTTPServer::HTTPServer(Function<void(Connection)>&& connectionHandler, Protocol protocol)
    104104    : m_listener(adoptNS(nw_listener_create(listenerParameters(protocol, nullptr).get())))
    105105    , m_protocol(protocol)
    … …  
    109109        nw_connection_set_queue(connection, dispatch_get_main_queue());
    110110        nw_connection_start(connection);
    111         connectionHandler(connection);
     111        connectionHandler(Connection(connection));
    112112    }).get());
    113113    startListening(m_listener.get());
    … …  
    137137}
    138138
    139 RetainPtr<dispatch_data_t> dataFromString(String&& s)
     139static RetainPtr<dispatch_data_t> dataFromString(String&& s)
    140140{
    141141    auto impl = s.releaseImpl();
    … …  
    146146}
    147147
    148 Vector<char> nullTerminatedRequest(dispatch_data_t content)
     148static Vector<char> vectorFromData(dispatch_data_t content)
    149149{
    150150    __block Vector<char> request;
    … …  
    153153        return true;
    154154    });
    155     request.append('\0');
    156155    return request;
    157156}
    158157
    159 void HTTPServer::respondToRequests(nw_connection_t connection, RefPtr<RequestData> requestData)
    160 {
    161     nw_connection_receive(connection, 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([connection = retainPtr(connection), requestData](dispatch_data_t content, nw_content_context_t context, bool complete, nw_error_t error) {
    162         if (error || !content)
     158void HTTPServer::respondToRequests(Connection connection, RefPtr<RequestData> requestData)
     159{
     160    connection.receiveHTTPRequest([connection, requestData] (Vector<char>&& request) {
     161        if (!request.size())
    163162            return;
    164 
    165163        requestData->requestCount++;
    166         auto request = nullTerminatedRequest(content);
    167164
    168165        const char* getPathPrefix = "GET ";
    169166        const char* postPathPrefix = "POST ";
    170167        const char* pathSuffix = " HTTP/1.1\r\n";
    171         const char* pathEnd = strstr(request.data(), pathSuffix);
     168        const char* pathEnd = strnstr(request.data(), pathSuffix, request.size());
    172169        ASSERT_WITH_MESSAGE(pathEnd, "HTTPServer assumes request is HTTP 1.1");
    173         const char* doubleNewline = strstr(request.data(), "\r\n\r\n");
    174         ASSERT_WITH_MESSAGE(doubleNewline, "HTTPServer assumes entire HTTP request is received at once");
    175170        size_t pathPrefixLength = 0;
    176171        if (!memcmp(request.data(), getPathPrefix, strlen(getPathPrefix)))
    … …  
    183178        ASSERT_WITH_MESSAGE(requestData->requestMap.contains(path), "This HTTPServer does not know how to respond to a request for %s", path.utf8().data());
    184179
    185         CompletionHandler<void()> sendResponse = [connection, context = retainPtr(context), path = WTFMove(path), requestData] {
    186             auto response = requestData->requestMap.get(path);
    187             if (response.terminateConnection == HTTPResponse::TerminateConnection::Yes) {
    188                 nw_connection_cancel(connection.get());
    189                 return;
    190             }
    191             StringBuilder responseBuilder;
    192             responseBuilder.append("HTTP/1.1 ");
    193             responseBuilder.appendNumber(response.statusCode);
    194             responseBuilder.append(' ');
    195             responseBuilder.append(statusText(response.statusCode));
    196             responseBuilder.append("\r\nContent-Length: ");
    197             responseBuilder.appendNumber(response.body.length());
    198             responseBuilder.append("\r\n");
    199             for (auto& pair : response.headerFields) {
    200                 responseBuilder.append(pair.key);
    201                 responseBuilder.append(": ");
    202                 responseBuilder.append(pair.value);
    203                 responseBuilder.append("\r\n");
    204             }
    205             responseBuilder.append("\r\n");
    206             responseBuilder.append(response.body);
    207             nw_connection_send(connection.get(), dataFromString(responseBuilder.toString()).get(), context.get(), true, ^(nw_error_t error) {
    208                 ASSERT(!error);
    209                 respondToRequests(connection.get(), requestData);
    210             });
    211         };
    212 
    213         if (auto* contentLengthBegin = strstr(request.data(), "Content-Length")) {
    214             size_t contentLength = atoi(contentLengthBegin + strlen("Content-Length: "));
    215             size_t headerLength = doubleNewline - request.data() + strlen("\r\n\r\n");
    216             constexpr size_t nullTerminationLength = 1;
    217             if (request.size() - nullTerminationLength - headerLength < contentLength) {
    218                 nw_connection_receive(connection.get(), 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([sendResponse = WTFMove(sendResponse)] (dispatch_data_t content, nw_content_context_t context, bool complete, nw_error_t error) mutable {
    219                     if (error || !content)
    220                         return;
    221                     sendResponse();
    222                 }).get());
    223                 return;
    224             }
    225         }
    226         sendResponse();
    227     }).get());
     180        auto response = requestData->requestMap.get(path);
     181        if (response.terminateConnection == HTTPResponse::TerminateConnection::Yes)
     182            return connection.terminate();
     183        StringBuilder responseBuilder;
     184        responseBuilder.append("HTTP/1.1 ", response.statusCode, ' ', statusText(response.statusCode), "\r\n");
     185        responseBuilder.append("Content-Length: ", response.body.length(), "\r\n");
     186        for (auto& pair : response.headerFields)
     187            responseBuilder.append(pair.key, ": ", pair.value, "\r\n");
     188        responseBuilder.append("\r\n");
     189        responseBuilder.append(response.body);
     190        connection.send(responseBuilder.toString(), [connection, requestData] {
     191            respondToRequests(connection, requestData);
     192        });
     193    });
    228194}
    229195
    … …  
    233199}
    234200
    235 NSURLRequest *HTTPServer::request() const
     201NSURLRequest *HTTPServer::request(const String& path) const
    236202{
    237203    NSString *format;
    238204    switch (m_protocol) {
    239205    case Protocol::Http:
    240         format = @"http://127.0.0.1:%d/";
     206        format = @"http://127.0.0.1:%d%s";
    241207        break;
    242208    case Protocol::Https:
    243209    case Protocol::HttpsWithLegacyTLS:
    244         format = @"https://127.0.0.1:%d/";
     210        format = @"https://127.0.0.1:%d%s";
    245211        break;
    246212    }
    247     return [NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:format, port()]]];
     213    return [NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:format, port(), path.utf8().data()]]];
     214}
     215
     216void Connection::receiveHTTPRequest(CompletionHandler<void(Vector<char>&&)>&& completionHandler, Vector<char>&& buffer) const
     217{
     218    nw_connection_receive(m_connection.get(), 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([connection = *this, completionHandler = WTFMove(completionHandler), buffer = WTFMove(buffer)](dispatch_data_t content, nw_content_context_t, bool, nw_error_t error) mutable {
     219        if (error || !content)
     220            return completionHandler({ });
     221        buffer.appendVector(vectorFromData(content));
     222        if (auto* doubleNewline = strnstr(buffer.data(), "\r\n\r\n", buffer.size())) {
     223            if (auto* contentLengthBegin = strnstr(buffer.data(), "Content-Length", buffer.size())) {
     224                size_t contentLength = atoi(contentLengthBegin + strlen("Content-Length: "));
     225                size_t headerLength = doubleNewline - buffer.data() + strlen("\r\n\r\n");
     226                if (buffer.size() - headerLength < contentLength)
     227                    return connection.receiveHTTPRequest(WTFMove(completionHandler), WTFMove(buffer));
     228            }
     229            completionHandler(WTFMove(buffer));
     230        } else
     231            connection.receiveHTTPRequest(WTFMove(completionHandler), WTFMove(buffer));
     232    }).get());
     233}
     234
     235void Connection::send(String&& message, CompletionHandler<void()>&& completionHandler) const
     236{
     237    nw_connection_send(m_connection.get(), dataFromString(WTFMove(message)).get(), NW_CONNECTION_DEFAULT_MESSAGE_CONTEXT, true, makeBlockPtr([completionHandler = WTFMove(completionHandler)](nw_error_t error) mutable {
     238        ASSERT(!error);
     239        if (completionHandler)
     240            completionHandler();
     241    }).get());
     242}
     243
     244void Connection::terminate() const
     245{
     246    nw_connection_cancel(m_connection.get());
    248247}
    249248
Note: See TracChangeset for help on using the changeset viewer.