Why is my version worse?

Question

linux

I'm learning the tinyproxy code . and here is the code. why is he so redundant? Or is my version redundant? here is the developer option.

#define SEGMENT_LEN (512)
#define MAXIMUM_BUFFER_LENGTH (128 * 1024)
ssize_t readline (int fd, char **whole_buffer)
{
        ssize_t whole_buffer_len;
        char buffer[SEGMENT_LEN];
        char *ptr;

        ssize_t ret;
        ssize_t diff;

        struct read_lines_s {
                char *data;
                size_t len;
                struct read_lines_s *next;
        };
        struct read_lines_s *first_line, *line_ptr;

        first_line =
            (struct read_lines_s *) safecalloc (sizeof (struct read_lines_s),
                                                1);
        if (!first_line)
                return -ENOMEM;

        line_ptr = first_line;

        whole_buffer_len = 0;
        for (;;) {
                ret = recv (fd, buffer, SEGMENT_LEN, MSG_PEEK);
                if (ret <= 0)
                        goto CLEANUP;

                ptr = (char *) memchr (buffer, '\n', ret);
                if (ptr)
                        diff = ptr - buffer + 1;
                else
                        diff = ret;

                whole_buffer_len += diff;

                /*
                 * Don't allow the buffer to grow without bound. If we
                 * get to more than MAXIMUM_BUFFER_LENGTH close.
                 */
                if (whole_buffer_len > MAXIMUM_BUFFER_LENGTH) {
                        ret = -ERANGE;
                        goto CLEANUP;
                }

                line_ptr->data = (char *) safemalloc (diff);
                if (!line_ptr->data) {
                        ret = -ENOMEM;
                        goto CLEANUP;
                }

                ret = recv (fd, line_ptr->data, diff, 0);
                if (ret == -1) {
                        goto CLEANUP;
                }

                line_ptr->len = diff;

                if (ptr) {
                        line_ptr->next = NULL;
                        break;
                }

                line_ptr->next =
                    (struct read_lines_s *)
                    safecalloc (sizeof (struct read_lines_s), 1);
                if (!line_ptr->next) {
                        ret = -ENOMEM;
                        goto CLEANUP;
                }
                line_ptr = line_ptr->next;
        }

        *whole_buffer = (char *) safemalloc (whole_buffer_len + 1);
        if (!*whole_buffer) {
                ret = -ENOMEM;
                goto CLEANUP;
        }

        *(*whole_buffer + whole_buffer_len) = '\0';

        whole_buffer_len = 0;
        line_ptr = first_line;
        while (line_ptr) {
                memcpy (*whole_buffer + whole_buffer_len, line_ptr->data,
                        line_ptr->len);
                whole_buffer_len += line_ptr->len;

                line_ptr = line_ptr->next;
        }

        ret = whole_buffer_len;

CLEANUP:
        do {
                line_ptr = first_line->next;
                if (first_line->data)
                        safefree (first_line->data);
                safefree (first_line);
                first_line = line_ptr;
        } while (first_line);

        return ret;
}

Well, I already understand how redundant my option is. the more data, the slower new memory is allocated with realloc. here is my version, it is not complete, I just gave the right piece.

ssize_t readline ( int fd, char **whole_buffer ) {
        ssize_t whole_buffer_len = 0;
        ssize_t ret;
        ssize_t pos = 0;
        char buffer[SEGMENT_LEN];

        *whole_buffer = calloc ( 0, sizeof ( char * ) );

        for(;;) {
                ret = recv ( fd, buffer, SEGMENT_LEN, 0 );
                if ( ret <= 0 ) goto CLEANUP;
                whole_buffer_len += ret;
                *whole_buffer = realloc ( *whole_buffer, whole_buffer_len );
                strncpy ( *whole_buffer + pos, buffer, ret );
                pos += ret;

        }
        ...
}

probably if the size is megabytes, then the next portion of data will look for a new memory area a few bytes more.

Answer 1

The original code reads the line (up to \n) and yours just reads the data into the buffer until the other end closes the socket.
Compare everything that is in that code and in yours, what is there and not in yours - that is not enough. For example, unpredictable results are possible (most likely, the disclosure of the contents of memory, since an uninitialized fragment remains with a piece of random data from the heap in the buffer) in the presence of ASCII (0) in water, realloc () errors are not processed (which is guaranteed to lead to memory corruption controlled data at a partially controlled address, which with a high probability leads to RCE, especially since heap spraying is possible), the code leads to DoS conditions with memory exhaustion, a text string is not terminated, etc. etc., i.e. only errors that are guaranteed to affect security in your fragment are at least 3x.
PS To be fair, the source code isn't great either. There are 2 times more kernel-to-memory system calls than actually required, and the size of the buffer being read into does not match the page size, which further slows down the process. To avoid this, it was necessary to add buffering in the user space. But at least there are no obvious security problems in it.

Answer 2

judging by the description of tinyxproxy, it can modify http headers. I think this is what splitting by line breaks and saving all these lines into a separate list is used for. Well, all this redundancy is just for this purpose.