views:

50

answers:

3

I'm trying to write a TCP client that does the following things:

1. Establish TCP connection to webserver
2. Accept GET request command from user's console
3. Client should get a reply back from webserver after each GET request.

I'm having a difficult time for the 3rd condition. I did not receive any response back from the webserver.

Here is my code:

    s = connectTCP(host, service);
 while (fgets(buf, sizeof(buf), stdin)) {
   buf[LINELEN-2]='\r';    /* ensure catridge return   */
   buf[LINELEN-1]='\n'; /* ensure line feed return  */
   buf[LINELEN] = '\0'; /* ensure line null-terminated */
   outchars = strlen(buf);
   (void) write(s, buf, outchars);
   printf("Start reading from socket...\n");
   fflush(stdout);
   while( (n = read(s, buf, LINELEN)) > 0) {
    buf[n] = '\0';  /* ensure null-terminated */
    (void) fputs( buf, stdout );
    fflush(stdout);
   }
 }
A: 

What sockets are you using? If your using (windows) berkley sockets you can use select to test for data and recv/recvfrom to get the data

Necrolis
i'm using berkley sockets in ubuntu. how to use select ?
tsubasa
I like a lot the Beej's guide to networking: http://beej.us/guide/bgnet/
Jaime Pardos
yeah, thats an excellent guide, especially for linux/unix users :)
Necrolis
+1  A: 

An HTTP request is terminated with two carriage-return–linefeed ("\r\n\r\n") sequences. The way you're constructing your request won't even ensure it has one. Maybe instead something like:

while (fgets(buf, sizeof(buf) - 3, stdin)) {
    size_t outsz = strlen(buf);
    if (outsz > 0 && buf[outsz - 1] == '\n')
        outsz--;
    strcpy(buf + outsz, "\r\n\r\n");
    write(s, buf, outsz + 4);
llasram
It does no harm but is not actually necessary on most servers (and you can even use single `\n` instead of `\r\n`).
kriss
@caf `fgets()` only fills up to `size - 1` chars with data and always appends a null, so `-3` is fine. The `write` includes the length of the data to send, so the string doesn't need to end up NUL-terminated. The `strcpy()` (or maybe `strcat()`) is probably better form though.
llasram
Agreed about not needing the nul-terminator for `write()` - could use `memcpy()` instead.
caf
A: 

There is several open loop-holes in this code that could lead to the problem. Really I bet on point 2.

1- does connectTCP really works ? Did it actually returned a working TCP socket. You could easily close that one checking for errors when calling write() instead of casting them to void.

2- if LINELEN is some constant like the size of an array, putting \r\n at the end of array won't be of much help if there is garbage between user input and the end of the array (chances are likely there is some 0). sizeof(buf) is the maximum legal value, not necessarilly the size of what the user will type.

3- double \r\n is not necessary, one '\n' (unix convention) would be enough.

4 - use of interactive user input for testing is usually not a good idea, it makes code more difficult to debug and test.

5 - beware writing at position n in you buffer if read buffer is full it could be one past the end of the buffer.

5 - if you want more than just sending one command and getting the answer you will have to write more elaborate code that manage both write() and read(). As another poster suggested select could be a good idea (probably simplet than thread). You could also work in blocking more alternating between command sent and received, but all of that that is probably for other question(s).

Below is a working piece of code (tested on Linux with gcc) that get an answer. Not that different, from you initial one, hope it will help you to close loopholes.

#include <sys/types.h>
#include <sys/socket.h>
#include <errno.h>
#include <fcntl.h>
#include <sys/un.h>
#include <netdb.h>
#include <arpa/inet.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>

enum {
    LINELEN = 1024
};

int main(){
    char buf[LINELEN];
    int n = 0;
    unsigned int option_len;
    int allow_reuse = 1;
    int sck = socket(PF_INET, SOCK_STREAM, 0);
    char * ip = "www.google.com";
    char * command = "GET http://www.google.com\n";

    /* connect to socket */
    struct sockaddr_in s;
    memset(&s, 0, sizeof(struct sockaddr_in));
    s.sin_family = AF_INET;
    s.sin_port = htons(80);
    s.sin_addr.s_addr = inet_addr(ip);
    if (s.sin_addr.s_addr == INADDR_NONE) {
        struct hostent *h = gethostbyname(ip);
        if (!h) {
            printf("DNS resolution failed for %s\n", ip);
            exit(0);
        }
        s.sin_addr.s_addr = *((int*)(*(h->h_addr_list)));
    }
    connect(sck, (struct sockaddr*)&s, sizeof(s));

    /* write command */
    printf("Write to socket...\n");
    write(sck, command, strlen(command));

    /* get answer */
    printf("Start reading from socket...\n");
    while((n = read(sck, buf, LINELEN-1)) > 0) {
       buf[n] = '\0';  /* ensure null-terminated */
       fputs( buf, stdout );
    }
}
kriss