Bug in C webserver

[wasab]

So I am trying to write a web server in C for my networking class. 

 

This is my source code 

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

#define PORT 6789

void send_html(int newSock, char buffer[], char buffer_tem[]);

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

    int sockfd, newSock;
    struct sockaddr_in serverAddr, newAddr;
    FILE* html_data = fopen("index.html", "r");
    if(html_data == NULL){
        perror("fopen error");
        exit(EXIT_FAILURE);
    }


    char buffer[2048] = "HTTP/1.1 200 OK\r\n\n";
    char buffer_tem[2048];
    while(fgets(buffer_tem, 2048, html_data)){
        strcat(buffer, buffer_tem);
    }

    pid_t child;
    sockfd = socket(AF_INET, SOCK_STREAM, 0);

    if(sockfd < 0){
        perror("socket creation");
        exit(EXIT_FAILURE);
    }

    printf("\n[+]Server socket created\n");

    memset(&serverAddr, 0, sizeof(serverAddr));

    serverAddr.sin_family = AF_INET;
    serverAddr.sin_port = htons(PORT);
    serverAddr.sin_addr.s_addr = INADDR_ANY;


    if(bind(sockfd, (struct sockaddr*) &serverAddr, sizeof(serverAddr)) < 0){
        perror("bind error");
        exit(EXIT_FAILURE);
    }

    printf("\n[+]Bind to port %d\n", PORT);

    if(listen(sockfd, 10) < 0){
        perror("Listening error");
        exit(EXIT_FAILURE);
    }

    while(1){
        socklen_t size = sizeof(newAddr);
        newSock = accept(sockfd, (struct sockaddr*) &newAddr, &size);
        if(newSock < 0){
            perror("Accpet error");
            exit(EXIT_FAILURE);
        }
        printf("\n[+]Accepted connection from %s:%d\n", inet_ntoa(newAddr.sin_addr), newAddr.sin_port);
        if((child = fork()) == 0){
            close(sockfd);
            recv(newSock, buffer_tem, 2048, 0);
            printf("\n[+]Got this request from client:\n%s\n", buffer_tem);
            send(newSock, buffer, sizeof(buffer), 0);
            //send_html(newSock, buffer, buffer_tem);
            close(newSock);
            exit(EXIT_SUCCESS);
        }
        close(newSock);
    }



    return 0;
}

void send_html(int newSock, char buffer[], char buffer_tem[]){
    send(newSock, buffer, sizeof(buffer), 0);
}

As you can see, it just reads data from a html file and then sends it to the client over tcp. This worked fine when I do not use the send_html function. 

 

 

 

 

Spoiler

However if I use the send_html function like this 

while(1){
        socklen_t size = sizeof(newAddr);
        newSock = accept(sockfd, (struct sockaddr*) &newAddr, &size);
        if(newSock < 0){
            perror("Accpet error");
            exit(EXIT_FAILURE);
        }
        printf("\n[+]Accepted connection from %s:%d\n", inet_ntoa(newAddr.sin_addr), newAddr.sin_port);
        if((child = fork()) == 0){
            close(sockfd);
            recv(newSock, buffer_tem, 2048, 0);
            printf("\n[+]Got this request from client:\n%s\n", buffer_tem);
            //send(newSock, buffer, sizeof(buffer), 0);
            send_html(newSock, buffer, buffer_tem);
            close(newSock);
            exit(EXIT_SUCCESS);
        }
        close(newSock);
    }

The socket doesn't send any data. 

 

 

 

 

Spoiler

What gives? send_html just does one thing which is send, it is the same as first version of the code above except it is done in another function. Why does it stop working if I use another function for sending the data? 

 

Can anyone give me any idea to what happening? 

[kompetenzbolzen]

I would assume sizeof() cant get the size of an array passed as an argument and thus returns zero as lenght. Remember that since arrays are just fancy notation for pointers, passing arrays does not copy them, it just passes the pointer.

[wasab]

14 minutes ago, kompetenzbolzen said:

I would assume sizeof() cant get the size of an array passed as an argument and thus returns zero as lenght. Remember that since arrays are just fancy notation for pointers, passing arrays does not copy them, it just passes the pointer.

 

Edit:

some alternatives:

Use the good ol' 

foo(char* bar, int barlen);

or use strlen() to determine the lenght of a string you are absolutely sure is correctly terminated.

you are right. Using  2048 works. But this is just a work around. Is there a way for me to pass the array correctly? 

 

Edit: without passing another parameter like int i meant. 

 

Edit: strlen is a good solution. 

Thanks for the help. 

[kompetenzbolzen]

 

foo(char* bar, int barlen);

or use strlen() to determine the lenght of a string you are absolutely sure is correctly terminated. Do that only of you can't do it any other way.

[kompetenzbolzen]

14 minutes ago, wasab said:

Edit: without passing another parameter like int i meant. 

 

Edit: strlen is a good solution. 

Thanks for the help. 

Also i believe recv() returns the number of read bytes. pass that. Please don't use strlen. I beg you. You're just asking for trouble...

[wasab]

27 minutes ago, kompetenzbolzen said:

Also i believe recv() returns the number of read bytes. pass that. Please don't use strlen. I beg you. You're just asking for trouble...

So not using strlen() is better? I am sure my buffer is properly terminated. I mean fprinting it looks fine. 

[kompetenzbolzen]

9 hours ago, wasab said:

So not using strlen() is better? I am sure my buffer is properly terminated. I mean fprinting it looks fine. 

Yes not using string functions is always better. since you know the length from recv() or at least know your buffer length, there is no need to rely on potentially dangerous functions. 

The String is only terminated because the data from the socket is. If your buffer is smaller than the data available, your buffer won't be terminated, as seen here:

char *buff = malloc(3);
int fd = open("test.txt", O_RDONLY); //test.txt has more than 3B of data in it
read (fd, buff, 3);

for(int i = 0; i < 3; i++)
	printf("%x ", buff[i]);

free(buff);

This outputs only 3 chars and no null termination.

[wasab]

7 hours ago, kompetenzbolzen said:

Yes not using string functions is always better. since you know the length from recv() or at least know your buffer length, there is no need to rely on potentially dangerous functions. 

The String is only terminated because the data from the socket is. If your buffer is smaller than the data available, your buffer won't be terminated, as seen here:

char *buff = malloc(3);
int fd = open("test.txt", O_RDONLY); //test.txt has more than 3B of data in it
read (fd, buff, 3);

for(int i = 0; i < 3; i++)
	printf("%x ", buff[i]);

free(buff);

This outputs only 3 chars and no null termination.

You got a good point there. I guess I can set the last byte to 0 to make sure it is null terminated but it kinda defeats the whole purpose.