[core] Fixed incorrect setting streamid in internal config in HS (last 4 characters)

[ethouris]

The problem:

The STREAMID (like all other string-type extensions) if it was set the length 509-512 (which is still legal), was rejected during setting of the config string because the length was taken as 512.

The string is passed in 4-byte chunks, which is then padded with zeros if the number of characters is not aligned to 4. The length to set the string was however calculated as the size of the extension data (which are 128 for the given sizes) multiplied by size of the field (4), which resulted to always 512, which exceeds the allowed size.

As the size of the actual string should be up to the first 0, the length should be taken directly by strlen because the length calculated out of blocklen may include the padding zeros, which makes the size wrong.

Additionally fixed bug in StringStorage: the condition for too big length is wrong.

[maxsharabayko]

A good case to extend socket option unit tests.

Possible test cases:

1. Try setting a 509-character string to SRTO_STREAMID. Expected to get a correct value after connection.
2. Try setting a 512-character string to SRTO_STREAMID.Expected to get a correct zero-terminated 512-character string via srt_getsockopt(.., SRTO_STREAMID).
3. Try setting a 513-character string. Expected: error setting the socket option.
4. Try setting a 1-character string to SRTO_STREAMID. Expected to get a correct value (one character) after connection.

[ethouris]

Just one more note: there's no problem with two other string-type extensions, congctl and packetfilter, because they are directly assigned to std::string, so this does use strlen internally.

[maxsharabayko]

Some unit tests.

// Try to set a wrong SRTO_STREAMID.
TEST_F(TestSocketOptions, StreamIDWrongLen)
{
    char buffer[648];
    for (size_t i = 0; i < sizeof buffer; ++i)
        buffer[i] = 'a' + i % 25;

    EXPECT_EQ(srt_setsockopt(m_caller_sock, 0, SRTO_STREAMID, buffer, 513), SRT_ERROR);
    EXPECT_EQ(srt_getlasterror(NULL), SRT_EINVPARAM);
}

// Try to set/get a 13-character string in SRTO_STREAMID.
// This tests checks that the StreamID is set to the correct size
// while it is transmitted as 16 characters in the Stream ID HS extension.
TEST_F(TestSocketOptions, StreamIDLen13)
{
    string stream_id_13 = "something1234";

    EXPECT_EQ(srt_setsockopt(m_caller_sock, 0, SRTO_STREAMID, stream_id_13.c_str(), stream_id_13.size()), SRT_SUCCESS);

    char buffer[648];
    int buffer_len = sizeof buffer;
    EXPECT_EQ(srt_getsockopt(m_caller_sock, 0, SRTO_STREAMID, &buffer, &buffer_len), SRT_SUCCESS);

    StartListener();
    const SRTSOCKET accepted_sock = EstablishConnection();

    // Check accepted socket inherits values
    for (SRTSOCKET sock : { accepted_sock })
    {
        for (size_t i = 0; i < sizeof buffer; ++i)
            buffer[i] = 'a';
        buffer_len = (int)(sizeof buffer);
        EXPECT_EQ(srt_getsockopt(sock, 0, SRTO_STREAMID, &buffer, &buffer_len), SRT_SUCCESS);
        EXPECT_EQ(buffer_len, 13);
    }

    ASSERT_NE(srt_close(accepted_sock), SRT_ERROR);
}

[maxsharabayko]

Setting SRTO_STREAMID on the listener socket as of SRT v1.4.2:

1. does not result in sending the StreamID extension message in the HS Conclusion Response;
2. does not result in an accepted socket having the StreamID value set.

TODO

• ❗ As of now setting SRTO_STREAMID on a listener socket result in this value derived on the accepted socket. The accepted socket must not derive this value from the listener socket. See the test case below.
• The existing behavior of ignoring the StreamID value on the listener socket is not well described in the docs. Only the Rendezvous case is mentioned.
• Maybe srt_setsockopt(.., SRTO_STREAMID,..) in case of a listener socket should return an error. To consider. 🤔

TEST_F(TestSocketOptions, StreamIDLenListener)
{
    string stream_id_13 = "something1234";

    EXPECT_EQ(srt_setsockopt(m_listen_sock, 0, SRTO_STREAMID, stream_id_13.c_str(), stream_id_13.size()), SRT_SUCCESS);

    char buffer[648];
    int buffer_len = sizeof buffer;
    EXPECT_EQ(srt_getsockopt(m_listen_sock, 0, SRTO_STREAMID, &buffer, &buffer_len), SRT_SUCCESS);

    StartListener();
    const SRTSOCKET accepted_sock = EstablishConnection();

    // Check accepted socket inherits values
    for (SRTSOCKET sock : { m_caller_sock, accepted_sock })
    {
        for (size_t i = 0; i < sizeof buffer; ++i)
            buffer[i] = 'a';
        buffer_len = (int)(sizeof buffer);
        EXPECT_EQ(srt_getsockopt(sock, 0, SRTO_STREAMID, &buffer, &buffer_len), SRT_SUCCESS);
        EXPECT_EQ(buffer_len, 0) << (sock == accepted_sock ? "ACCEPTED" : "LISTENER");
    }

    ASSERT_NE(srt_close(accepted_sock), SRT_ERROR);
}

[stevomatthews]

Minoir edit