Estoy haciendo un chat simple que tiene que enviar un mensaje de texto hecho por un miembro a todos los demás miembros. El formato del mensaje que todo el mundo tiene que recibir es "[IP]: hello!". También el servidor tiene que notificar a todo el mundo cuando alguien se conecta o se desconecta: "[IP] se ha conectado" y "[IP] está doosconnected" respectivamente.

Aquí hay un fragmento de código que implementa esta función del servidor. Usted puede comenzar a buscar desde la línea que tiene "EL PROBLEMA ES AQUI" comentario:

while (true)
{

// select() for reading

static constexpr int bufferSize = 1024;
static char buffer[bufferSize];

// this is for getting IP-address of sender
static sockaddr_in sockAddr;
static socklen_t sockAddrSize;

if (FD_ISSET(masterSocket, &set)) // masterSocket is a socket that establishes connections
{
    sockAddrSize = sizeof(sockAddr);
    int slaveSocket = accept(masketSocket, &sockAddr, &sockAddrSize); // slaveSocket is a client socket

    // setting a slaveSocket non-blocking

    sprintf(buffer, "[%d.%d.%d.%d] has connected\n", 
        (sockAddr.sin_addr.s_addr & 0x000000FF), 
        (sockAddr.sin_addr.s_addr & 0x0000FF00) >> 8, 
        (sockAddr.sin_addr.s_addr & 0x00FF0000) >> 16, 
        (sockAddr.sin_addr.s_addr & 0xFF000000) >> 24);

    for (const auto &socket : slaveSockets)
        send(socket, buffer, strlen(buffer), MSG_NOSIGNAL);

    slaveSockets.insert(slaveSocket);
}

for (const auto &socket : slaveSockets)
{
    if (FD_ISSET(socket, &set))
        continue;

    static int recvSize = recv(socket, buffer, bufferSize, MSG_NOSIGNAL);

    if (recvSize == 0 && errno != EAGAIN)
    {
        sockAddrSize = sizeof(sockAddr);
        getsockname(socket, (sockaddr *) &sockAddr, &sockAddrSize);
        sprintf(buffer, "[%d.%d.%d.%d] has disconnected\n", 
                    (sockAddr.sin_addr.s_addr & 0x000000FF), 
                    (sockAddr.sin_addr.s_addr & 0x0000FF00) >> 8, 
                    (sockAddr.sin_addr.s_addr & 0x00FF0000) >> 16, 
                    (sockAddr.sin_addr.s_addr & 0xFF000000) >> 24);

        shutdown(socket, SHUT_RDWR);
        close(socket);
        slaveSockets.erase(socket);

        for (const auto &socket : slaveSockets)
            send(socket, buffer, strlen(buffer), MSG_NOSIGNAL);
    }
    else if (recvSize > 0) // THE PROBLEM IS HERE
    {
        static char reply[bufferSize];
        sockAddrSize = sizeof(&sockAddr);
        getsocklen(socket, (sockaddr *) &sockAddr, &sockAddrSize);
        sprintf(reply, "[%d.%d.%d.%d]: %s\n",
            (sockAddr.sin_addr.s_addr & 0x000000FF),
            (sockAddr.sin_addr.s_addr & 0x0000FF00) >> 8,
            (sockAddr.sin_addr.s_addr & 0x00FF0000) >> 16,
            (sockAddr.sin_addr.s_addr & 0xFF000000) >> 24,
            buffer);

        int senderSocket = socket;
        for (const auto &socket : slaveSockets)
        {
            if (socket == senderSocket)
                continue;

            send(socket, reply, strlen(reply), MSG_NOSIGNAL); // even tried the "strlen(reply) + 1"
        }
    }
}

}

El problema es que los receptores tienen cada mensaje dado salida incorrectamente: se genera completamente pero también tiene el final de los valores antiguos del búfer al final. Por ejemplo:

El cliente A se ha conectado.

El cliente B se ha conectado. El cliente A ha recibido "[127.0.0.1] ha conectado".

El cliente A ha enviado "hola". El cliente B recibió "[127.0.0.1]: hello-n0.1] ha conectado a n".

El cliente B ha enviado "¿qué pasa?". El cliente A ha recibido "[127.0.0.1]: ¿qué pasa?

El cliente A se ha desconectado. El cliente B ha recibido "[127.0.0.1] se ha desconectado".

Como puede ver la información de conexión/desconexión siempre se genera correctamente, pero el chat funciona mal: contiene partes de la información de conexión/disonción en su extremo.

Sinceramente creo que uso los buffers correctamente y no puedo entender lo que estoy haciendo mal.

1
ghostinecatnewyear 3 nov. 2019 a las 16:42

2 respuestas

La mejor respuesta

buffer no es una cadena C terminada en nulo después de que recv regrese. Esto es lógico: ¿qué sucede si transfiere datos binarios? Luego, deseará recv bytes exactos (longitud del mensaje) y no agregará bytes cero.

Tenga en cuenta que el envío del byte nulo final en send es incorrecto: su receptor depende del remitente para agregar este byte cero, pero si el remitente es malicioso, entonces no puede agregar el byte cero y causar todo tipo de errores y vulnerabilidades, incluidos ataques DoS y ejecución remota de código.

Aún puede confiar en que el remitente agregue un byte cero, pero luego debe pasar bufferSize-1 como longitud del búfer a recv, y después de una llamada a recv configure el reply[bufferSize-1]=0. Pero tal vez todavía no sea lo mejor que se puede hacer: una de las muchas otras opciones es pasar una "longitud de mensaje" como un entero sin codificar de 32 bits, verificar la longitud máxima (por ejemplo, ningún mensaje es mayor que 1024 caracteres, y si es así, no reciba nada y simplemente cierre el socket), y recv exactamente los bytes de "longitud de mensaje" pasados al búfer. Aún necesitará agregar el byte nulo de terminación si tiene la intención de usar el búfer como una cadena de estilo C.

Editar: ¡IMPORTANTE! Si usa TCP (SOCK_STREAM), use siempre la longitud del mensaje: el mensaje podría (y algún día) será leído por recv en fragmentos. Absolutamente debes concatenarlos en un mensaje completo por ti mismo.

2
smitsyn 3 nov. 2019 a las 14:13

Solo para agregar a la respuesta de smitsyn, debe usar recvSize (que es mayor que 0 en la rama con el problema, ya que ha recibido algo de uno de los clientes) para configurar el '\ 0' búfer interior.

Sus variables estáticas se inicializaron a 0 (predeterminado) por lo que su búfer contenía un '\ 0' inmediatamente después del mensaje más largo que recibió (ya que el resto se sobrescribió) y tuvo suerte de que sprintf Lo encontré y no lo busqué en otro lugar de la memoria de su programa (incluso fuera de los límites).

Algo en este sentido debería hacerlo funcionar (para un caso simple):

else if (recvSize > 0) // THE PROBLEM IS HERE
    {
     ...

     buffer[recvSize] = '\0'; // of course make sure it fits! could use a std::min(recvSize, bufferSize - 1)
     sprintf(reply, "[%d.%d.%d.%d]: %s\n",
            (sockAddr.sin_addr.s_addr & 0x000000FF),
            (sockAddr.sin_addr.s_addr & 0x0000FF00) >> 8,
            (sockAddr.sin_addr.s_addr & 0x00FF0000) >> 16,
            (sockAddr.sin_addr.s_addr & 0xFF000000) >> 24,
            buffer); // now this print should work as expected 

Editar: porque no puedo publicar comentarios :(

1
ben10 3 nov. 2019 a las 14:23