SrsRtcTcpConn::read_packet should use read_fully instead of read

[sandro-qiang]

Describe the bug
read_packet use skt_->read to read packet length, not skt_->read_fully. for client not using tcp_no_delay option, this may lead to read wrong packet length.

Version
5 and 6

[winlinvip]

This is indeed an error. Can you submit a PR (Pull Request)?

TRANS_BY_GPT4

[sandro-qiang]

I'm a bit busy now, can you just fix this? only a single line.

[chundonglinlin]

I've made some changes, please review, @sandro-qiang. Could you explain how to test the client (can you provide a client demo that does not use the tcp_no_delay option for streaming webrtc over tcp)? I've only done a simple test on the web player.

srs_error_t SrsRtcTcpConn::read_packet(char* pkt, int* nb_pkt)
{
    srs_error_t err = srs_success;

    // Read length in 2 bytes @doc: https://www.rfc-editor.org/rfc/rfc4571#section-2
    ssize_t nread = 0; uint8_t b[2];
    if((err = skt_->read((char*)b, sizeof(b), &nread)) != srs_success) {
        return srs_error_wrap(err, "rtc tcp conn read len");
    }

    uint16_t npkt = uint16_t(b[0])<<8 | uint16_t(b[1]);
    if (npkt > *nb_pkt) {
        return srs_error_new(ERROR_RTC_TCP_SIZE, "invalid size=%u exceed %d", npkt, *nb_pkt);
    }

    // Read a RTC pkt such as STUN, DTLS or RTP/RTCP
    if((err = skt_->read(pkt, npkt, &nread)) != srs_success) {
        return srs_error_wrap(err, "rtc tcp conn read body");
    }

    *nb_pkt = npkt;

    return err;
}

TRANS_BY_GPT4

[winlinvip]

Please submit a PR (Pull Request) directly, and let's discuss it on the PR.

TRANS_BY_GPT4

[sandro-qiang]

I've made some changes, please review, @sandro-qiang. Could you explain how to test the client (can you provide a client demo that does not use the tcp_no_delay option for streaming webrtc over tcp)? I've only done a simple test on the web player.

srs_error_t SrsRtcTcpConn::read_packet(char* pkt, int* nb_pkt)
{
    srs_error_t err = srs_success;

    // Read length in 2 bytes @doc: https://www.rfc-editor.org/rfc/rfc4571#section-2
    ssize_t nread = 0; uint8_t b[2];
    if((err = skt_->read((char*)b, sizeof(b), &nread)) != srs_success) {
        return srs_error_wrap(err, "rtc tcp conn read len");
    }

    uint16_t npkt = uint16_t(b[0])<<8 | uint16_t(b[1]);
    if (npkt > *nb_pkt) {
        return srs_error_new(ERROR_RTC_TCP_SIZE, "invalid size=%u exceed %d", npkt, *nb_pkt);
    }

    // Read a RTC pkt such as STUN, DTLS or RTP/RTCP
    if((err = skt_->read(pkt, npkt, &nread)) != srs_success) {
        return srs_error_wrap(err, "rtc tcp conn read body");
    }

    *nb_pkt = npkt;

    return err;
}

TRANS_BY_GPT4

Full demo involve a lot of logic, connect -> stun binding -> dtls handshake -> rtp/rtcp. Base on srs source code, the simplest demo I can see is make a client program only do connect -> stun, but make sure stun is not a binding request, so srs will ignore this packet. Make the dummy stun packet size vary(body has random chunks). The default create_socket function has no tcp_no_delay option on, so any tcp connect code can use(I found this bug using boost::asio, the boost::asio::async_connect function).