microsoft / msquic

Cross-platform, C implementation of the IETF QUIC protocol, exposed to C, C++, C# and Rust.
MIT License
4.07k stars 535 forks source link

When I send data at a high speed, the data will be wrong #4546

Open Anveena opened 2 months ago

Anveena commented 2 months ago

Describe the bug

Simply sending 512*1024 0x00 data each time, with a 32-byte message header, about dozens to 200 times, through wireshark and my service, you can see that the message header has changed, and I make sure that the data I give to StreamSend has not changed. My setting:

    Settings.IdleTimeoutMs = 1000 * 60;
    Settings.IsSet.IdleTimeoutMs = 1;

//In fact, these lines have no effect on the reproduction of the bug, and problems will still occur.
    Settings.TlsClientMaxSendBuffer = 1024 * 1024 * 2;
    Settings.IsSet.TlsClientMaxSendBuffer = 1;
    Settings.SendBufferingEnabled = 1;

Affected OS

Additional OS information

No response

MsQuic version

main

Steps taken to reproduce bug

image The structure of the first 12 characters is as follows

typedef struct foo{
    unsigned char m;
    unsigned char type;
    unsigned short subType;
    unsigned int timestamp;
    unsigned int contentLen;
};

image

        printf("[strm][%p] Send data...,buffer ptr[%p],buf->buffer->len:%d\n\tmsgType:%d,subType:%d,contentLen:%d\n",
               Stream,
               buf,
               buf->buffer->Length,
               ((struct A*) buf->buffer->Buffer)->type,
               ((struct A*) buf->buffer->Buffer)->subType,
               ((struct A*) buf->buffer->Buffer)->contentLen);
        if (QUIC_FAILED(Status = gQUIC->StreamSend(Stream, buf->buffer, 1, QUIC_SEND_FLAG_NONE, buf))) {
            printf("StreamSend failed, 0x%x!\n", Status);
            goto Error2;
        }

This should indicate that my buffer has not changed, but wireshark and my service have changed. The data sent not only loses the first byte at the beginning, but also has some incredible changes in the subsequent bytes.

Expected behavior

image

Actual outcome

image

Additional details

No response

nibanks commented 2 months ago

Are you using send buffering or not. The settings you have there show you set it to 1, but not that you set IsSet?

But are you sure you're not changing or deleting the send buffers you give to MsQuic before you get the send completions? Just because the send function call returns doesn't mean we're fine with the buffer.

Finally, why are you calling your variable gQUIC? 😂 This isn't Google QUIC...

y11en commented 2 months ago

gQUIC

Maybe it means global variables.

😄

Anveena commented 2 months ago

Are you using send buffering or not. The settings you have there show you set it to 1, but not that you set IsSet?

No,Whether I set it or not, it does not affect the result.

But are you sure you're not changing or deleting the send buffers you give to MsQuic before you get the send completions? Just because the send function call returns doesn't mean we're fine with the buffer.

In fact, I waited for the Sent Event callback before sending the new data, and the data was previously filled and was not modified in the subsequent io test.

Finally, why are you calling your variable gQUIC? 😂 This isn't Google QUIC...

Haha, mainly because it is a simple IO test, the logic may be poor. g means global.

nibanks commented 2 months ago

Based on previous experience, this is caused by the app messing with the buffers passed into StreamSend before it gets a send complete event. We have various tests that verify the bytes are correctly sent, so I have pretty high confidence there isn't a bug in MsQuic.

Anveena commented 2 months ago

This is the relevant part of the code, I didn't have any chance to modify the buffer:

// Create some fake data
    MESSAGE_HEADER *header = malloc(sizeof(MESSAGE_HEADER));
    header->m = 0xcc;
    header->type = 3;
    header->subType = 4;
    //#define FAKE_DATA_LENGTH (1024 * 512)
    header->contentLen = FAKE_DATA_LENGTH;
    // prepare fake data
    EZ_BUFFER_FOR_QUIC *first = getUsableBuffer(buf);
    setBufferUsable(first, true);
    EZ_BUFFER_FOR_QUIC *tmp = first;
    while (true) {
        waitForBuffer(tmp);
        setBufferUsable(tmp, true);
        tmp->buffer->Length = sizeof(MESSAGE_HEADER) + FAKE_DATA_LENGTH;
        memcpy(tmp->buffer->Buffer, header, sizeof(MESSAGE_HEADER));
        printf("[strm][%p] Prepare data...,buffer ptr[%p],buf->buffer->len:%d\n\tmsgType:%d,subType:%d,contentLen:%u\n",
               Stream,
               tmp,
               tmp->buffer->Length,
               ((MESSAGE_HEADER *) tmp->buffer->Buffer)->type,
               ((MESSAGE_HEADER *) tmp->buffer->Buffer)->subType,
               ((MESSAGE_HEADER *) tmp->buffer->Buffer)->contentLen);
        tmp = tmp->next;
        if (tmp == first) {
            break;
        }
    }

// Then start sending
while (true) {
        buf = getUsableBuffer(buf);
        setBufferUsable(buf, false);
        printf("[strm][%p] Send data...,buffer ptr[%p],buf->buffer->len:%d\n\tmsgType:%d,subType:%d,contentLen:%d\n",
               Stream,
               buf,
               buf->buffer->Length,
               ((MESSAGE_HEADER *) buf->buffer->Buffer)->type,
               ((MESSAGE_HEADER *) buf->buffer->Buffer)->subType,
               ((MESSAGE_HEADER *) buf->buffer->Buffer)->contentLen);
        if (QUIC_FAILED(Status = gQUIC->StreamSend(Stream, buf->buffer, 1, QUIC_SEND_FLAG_NONE, buf))) {
            printf("StreamSend failed, 0x%x!\n", Status);
            goto Error2;
        }
    }

//  This is my handling in ClientStreamCallback
        case QUIC_STREAM_EVENT_SEND_COMPLETE:
//
// A previous StreamSend call has completed, and the context is being
// returned back to the app.
//

            EZ_BUFFER_FOR_QUIC *buf = Event->SEND_COMPLETE.ClientContext;
            setBufferUsable(buf, true);
            printf("[strm][%p] Data sent\n", Stream);
            break;

As mentioned above, I did not modify any buffers during the initial transmission, and I locked the buffer before sending and released the buffer after sending,strictly .

If I have to say what is different between me and test, it should be that my client uses the certificate

    QUIC_CREDENTIAL_CONFIG CredConfig;
    memset(&CredConfig, 0, sizeof(CredConfig));
    CredConfig.Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE;
    CredConfig.Flags = QUIC_CREDENTIAL_FLAG_CLIENT | QUIC_CREDENTIAL_FLAG_SET_CA_CERTIFICATE_FILE;
    CredConfig.CaCertificateFile = TLS_PATH_CA_CERT;

    QUIC_CERTIFICATE_FILE CertFile;
    CertFile.CertificateFile = TLS_PATH_CLIENT_CERT;
    CertFile.PrivateKeyFile = TLS_PATH_CLIENT_KEY;
    CredConfig.CertificateFile = &CertFile;
    if (QUIC_FAILED(Status = gQUIC->ConfigurationLoadCredential(Configuration, &CredConfig))) {
        printf("ConfigurationLoadCredential failed, 0x%x\n", Status);
        return -1;
    }

If it is a problem on my side, it should be related to the size of my circular buffer, but in fact, the time when the abnormal data is generated is very random. Every time I run it, the problematic data appears randomly, and the picture in the previous comment can also prove that there is no problem with the data I printed in the line before StreamSend.

image