1

Can anybody give me a hand trying to implement the following server and client?:

The server:

#include <stdlib.h>
#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdio.h>
#include <string.h>

int main(void) {
    int sock = socket(AF_INET, SOCK_STREAM, 0);

    struct sockaddr_in serv_addr = { 0 };
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_addr.s_addr = INADDR_ANY;
    serv_addr.sin_port = htons(1234);
    bind(sock, (struct sockaddr *)&serv_addr, sizeof(serv_addr));

    listen(sock, 128);

    struct sockaddr_in cli_addr = { 0 };
    socklen_t cli_addrlen = sizeof(cli_addr);
    int acc_sock = accept(sock, (struct sockaddr *)&cli_addr, &cli_addrlen);
    printf("[+] Connected \n");
    char buf[1024];
    ssize_t nread;
    memset(buf, 0, sizeof(buf));
    int a;
    while (1) {
        nread = read(0, buf, 1024);
        write(acc_sock, buf, nread);
        memset(buf, 0, sizeof(buf));

        while ((read(acc_sock, buf, 1024)) != 0) {
            printf("%s", buf);
            memset(buf, 0, sizeof(buf));
        }
    }
}

All the servers does is scanning a command from stdin and send it to the client via sockets. Then scans the client response and prints it out to stdout.

The client:

#include <stdlib.h>
#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdio.h>
#include <string.h>

int main(int argc, const char *argv[]) {
    int sock = socket(AF_INET, SOCK_STREAM, 0);
    struct sockaddr_in serv = { 0 };
    char buf[1024];
    char command[1024];
    memset(buf, 0, sizeof(buf));
    memset(command, 0, sizeof(command));
    int nread;
    FILE *in;
    extern FILE *popen();

    serv.sin_family = AF_INET;
    serv.sin_port = htons(atoi(argv[1]));
    serv.sin_addr.s_addr = inet_addr("127.0.0.1");
    int a;  
    connect(sock, (struct sockaddr*)&serv, sizeof(serv));
    while (1) {
        nread = read(sock, buf, 1024);
        in = popen(buf, "r");
        while ((fgets(command, sizeof(command), in)) != NULL) {;
            write(sock, command, sizeof(command));
        }
        memset(buf, 0, sizeof(buf));
    }
    return 0;
}

Essentially the client receives the command scanned by the server, executes it using popen(), and sends the contents of the command line by line until NULL.

The problem is that the code suddenly stops working just after the first command. the transmission of the command and the command's output is satisfactory, however after printing the output of the first command the program just stops working. I think is a problem with fgets(), however I could be wrong. Is there any solutions for this problem?

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 2
    `pclose()` is the proper close to go with `popen()` – John Hascall Feb 16 '16 at 22:05
  • 3
    Also SOCK_STREAM does not recognize any sort of message boundaries. Your `nread = read(sock, buf, 1024);` in the client could read anywhere from 1 to 1024 characters. And it won't `\0` terminate what it read either. – John Hascall Feb 16 '16 at 22:07
  • 3
    And finally, syscalls, as a rule, return useful values. Often Error/OK, you should check these return values. – John Hascall Feb 16 '16 at 22:08
  • hi john thanks for the feedback, could you be a bit more concrete about the solution approach you are encouraging? – Ñach Huidobro Feb 16 '16 at 22:10
  • As mentioned in the answer from user58697, you need some way for each end to tell the other where the boundaries are. SOCK_STREAMs are just streams of bytes that go on forever until closed, there are no implicit boundaries and you can't tell how many bytes any read or write will return/send (other than it will be <= your maxsize). You need to figure out how you want each end to signal the other than "this message is done". – John Hascall Feb 16 '16 at 22:43
  • `read()` returns `0` only when it completes successfully without transferring any bytes, either because you requested zero bytes, or because the file is positioned at its end. For a file associated with the input side of a socket, normal end-of-file occurs only when the socket has been closed at the other end. In particular, the absence of any bytes *currently* available to read does not in itself imply end-of-file; after all, more may be sent in the future, and may even be on their way already. If you call `read()` under such circumstances then it will block. – John Bollinger Feb 16 '16 at 22:43
  • The normal orientation for an app like this is that a client connects to a server and the client feeds the commands [read from stdin] to the server [which does popen] and feeds back the results. That's how `ssh` works. Your orientation is reversed. What you've got is you fire `sshd` and wait for `ssh` to connect and then `sshd` sends commands to `ssh`. Are you sure that's what you want [and why?]. In other words, the loops in the respective sides should be switched. – Craig Estey Feb 16 '16 at 23:05
  • Since you can only pass one line to `popen()` what the client needs to do is read the socket until it finds a newline, then it can call popen with the accumulated command. Sending the data back is a bit harder. How do you let the server know you have sent everything? One method would be with a count byte. You grab upto 127 bytes of popen's output, send a count byte (value between 1 and 127) first then that many bytes of data. Server reads the count, knows how much data follows. When the popen says EOF, and you have no more data to send, call `pclose()` and send a 0 count byte. – John Hascall Feb 16 '16 at 23:05
  • As I'm guessing this is a course assignment, I'm reluctant to go too much further... And, Craig Estey is spot on with the unusualness of your client and server roles, but perhaps that is actually what you were asked to do... – John Hascall Feb 16 '16 at 23:07

2 Answers2

2

Caveat: This may [or may not] fit your needs because I reversed the sense of the client and server loops in the corrected code below. As I had mentioned in my comment above:

The normal orientation for an app like this is that a client connects to a server and the client feeds the commands [read from stdin] to the server [which does popen] and feeds back the results. That's how ssh works. Your orientation is reversed. What you've got is you fire sshd and wait for ssh to connect and then sshd sends commands to ssh. In other words, the loops in the respective sides should be switched.

Reversing this was the only way things made sense to me. If the reversal doesn't work [well] for your desired use case, the code below may still give you some ideas.

I solved the hang problem by introducing the concept of a flag character to denote end-of-output. I borrowed this concept from the PPP [point-to-point] protocol over RS-232.

The flag character is just a given value (e.g. 0x10) that is not likely to be part of normal data. Since your data is most likely ascii or utf-8, any [unused] chars in the range 0x00-0x1F may be used (i.e. don't use tab, cr, newline, etc).

If you need to transmit the flag character (i.e. your data has to be the full binary range 0x00-0xFF), I've included some packet encode/decode routines that implement the escape codes used in PPP above. I've coded them, but did not actually hook them in. In this case, the flag [and escape] chars can be any binary value [usually 0xFF and 0xFE respectively].

For simplicity, I combined both sides into a single .c file. Invoke the server with -s [first].

Anyway, here's the tested code [please pardon the gratuitous style cleanup]:

// inetpair/inetpair -- server/client communication

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>

typedef unsigned char byte;

#define BUFMAX          1024
int port = 1234;
int opt_svr;
int opt_debug;

#define FLAG            0x10
#define ESC             0x11
#define ESC_FLAG        0x01
#define ESC_ESC         0x02

#define dbgprt(_fmt...) \
    do { \
        if (opt_debug) \
            printf(_fmt); \
    } while (0)

// client
int
client(void)
{
    int sock;
    struct sockaddr_in serv = { 0 };
    char *cp;
    char buf[BUFMAX + 1];
    int nread;
    int flag;
    int exitflg;

    sock = socket(AF_INET, SOCK_STREAM, 0);

    serv.sin_family = AF_INET;
    serv.sin_port = htons(port);
    serv.sin_addr.s_addr = inet_addr("127.0.0.1");

    connect(sock, (struct sockaddr *) &serv, sizeof(serv));

    while (1) {
        cp = fgets(buf,BUFMAX,stdin);
        if (cp == NULL)
            break;

        exitflg = (strcmp(buf,"exit\n") == 0);

        // send the command
        nread = strlen(buf);
        write(sock, buf, nread);

        if (exitflg)
            break;

        while (1) {
            dbgprt("client: PREREAD\n");
            nread = read(sock, buf, 1024);
            dbgprt("client: POSTREAD nread=%d\n",nread);
            if (nread <= 0)
                break;

            cp = memchr(buf,FLAG,nread);
            flag = (cp != NULL);
            if (flag)
                nread = cp - buf;

            write(1,buf,nread);

            if (flag)
                break;
        }
    }

    close(sock);

    return 0;
}

// server
int
server(void)
{
    struct sockaddr_in serv_addr = { 0 };
    int sock;
    int acc_sock;
    char buf[BUFMAX + 1];
    char command[BUFMAX + 1];
    ssize_t nread;
    FILE *pin;
    FILE *xfin;
    char *cp;
    struct sockaddr_in cli_addr = { 0 };

    opt_debug = ! opt_debug;

    dbgprt("[+] Starting\n");

    sock = socket(AF_INET, SOCK_STREAM, 0);

    serv_addr.sin_family = AF_INET;
    serv_addr.sin_addr.s_addr = INADDR_ANY;
    serv_addr.sin_port = htons(port);
    bind(sock, (struct sockaddr *) &serv_addr, sizeof(serv_addr));

    listen(sock, 128);

    while (1) {
        socklen_t cli_addrlen = sizeof(cli_addr);

        dbgprt("[+] Waiting for connection\n");
        acc_sock = accept(sock,(struct sockaddr *)&cli_addr,&cli_addrlen);

        dbgprt("[+] Connected\n");

        xfin = fdopen(acc_sock,"r");

        while (1) {
            dbgprt("[+] Waiting for command\n");

            cp = fgets(buf,BUFMAX,xfin);
            if (cp == NULL)
                break;

            cp = strchr(buf,'\n');
            if (cp != NULL)
                *cp = 0;

            dbgprt("[+] Command '%s'\n",buf);

            if (strcmp(buf,"exit") == 0)
                break;

            pin = popen(buf, "r");
            while (1) {
                cp = fgets(command, BUFMAX, pin);
                if (cp == NULL)
                    break;
                nread = strlen(command);
                write(acc_sock, command, nread);
            }
            pclose(pin);

            command[0] = FLAG;
            write(acc_sock,command,1);
        }

        fclose(xfin);
        close(acc_sock);

        dbgprt("[+] Disconnect\n");
    }
}

// packet_encode -- encode packet
// RETURNS: (outlen << 1)
int
packet_encode(void *dst,const void *src,int srclen)
{
    const byte *sp = src;
    byte *dp = dst;
    const byte *ep;
    byte chr;
    int dstlen;

    // encode packet in manner similar to PPP (point-to-point) protocol does
    // over RS-232 line

    ep = sp + srclen;
    for (;  sp < ep;  ++sp) {
        chr = *sp;

        switch (chr) {
        case FLAG:
            *dp++ = ESC;
            *dp++ = ESC_FLAG;
            break;

        case ESC:
            *dp++ = ESC;
            *dp++ = ESC_ESC;
            break;

        default:
            *dp++ = chr;
            break;
        }
    }

    dstlen = dp - (byte *) dst;
    dstlen <<= 1;

    return dstlen;
}

// packet_decode -- decode packet
// RETURNS: (outlen << 1) | flag
int
packet_decode(void *dst,const void *src,int srclen)
{
    const byte *sp = src;
    byte *dp = dst;
    const byte *ep;
    byte chr;
    int flag;
    int dstlen;

    // decode packet in manner similar to PPP (point-to-point) protocol does
    // over RS-232 line

    ep = sp + srclen;
    flag = 0;
    while (sp < ep) {
        chr = *sp++;

        flag = (chr == FLAG);
        if (flag)
            break;

        switch (chr) {
        case ESC:
            chr = *sp++;

            switch (chr) {
            case ESC_FLAG:
                *dp++ = FLAG;
                break;
            case ESC_ESC:
                *dp++ = ESC;
                break;
            }
            break;

        default:
            *dp++ = chr;
            break;
        }
    }

    dstlen = dp - (byte *) dst;
    dstlen <<= 1;

    if (flag)
        dstlen |= 0x01;

    return dstlen;
}

int
main(int argc, char **argv)
{
    char *cp;

    --argc;
    ++argv;

    for (;  argc > 0;  --argc, ++argv) {
        cp = *argv;
        if (*cp != '-')
            break;

        switch (cp[1]) {
        case 'd':
            opt_debug = 1;
            break;

        case 'P':
            port = atoi(cp + 2);
            break;

        case 's':
            opt_svr = 1;
            break;
        }
    }

    if (opt_svr)
        server();
    else
        client();

    return 0;
}
Craig Estey
  • 30,627
  • 4
  • 24
  • 48
1

The client never closes sock. Therefore the server's loop

    while ((read(acc_sock, buf, 1024)) != 0) {
        printf("%s", buf);
        memset(buf, 0, sizeof(buf));
    }

never terminates. You need some mechanism to inform server that all the command's output has been sent. Maybe something similar to HTTP chunked transfer encoding.

user58697
  • 7,808
  • 1
  • 14
  • 28