diff --git a/src/core/linebuf.c b/src/core/linebuf.c index 7930d19a..c197e383 100644 --- a/src/core/linebuf.c +++ b/src/core/linebuf.c @@ -43,7 +43,18 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); * @ret line Buffered line, or NULL if no line ready to read */ char * buffered_line ( struct line_buffer *linebuf ) { - return ( linebuf->ready ? linebuf->data : NULL ); + char *line = &linebuf->data[ linebuf->len ]; + + /* Fail unless we have a newly completed line to retrieve */ + if ( ( linebuf->len == 0 ) || ( linebuf->consumed == 0 ) || + ( *(--line) != '\0' ) ) + return NULL; + + /* Identify start of line */ + while ( ( line > linebuf->data ) && ( line[-1] != '\0' ) ) + line--; + + return line; } /** @@ -52,10 +63,11 @@ char * buffered_line ( struct line_buffer *linebuf ) { * @v linebuf Line buffer */ void empty_line_buffer ( struct line_buffer *linebuf ) { + free ( linebuf->data ); linebuf->data = NULL; linebuf->len = 0; - linebuf->ready = 0; + linebuf->consumed = 0; } /** @@ -76,16 +88,13 @@ void empty_line_buffer ( struct line_buffer *linebuf ) { * should call empty_line_buffer() before freeing a @c struct @c * line_buffer. */ -ssize_t line_buffer ( struct line_buffer *linebuf, - const char *data, size_t len ) { +int line_buffer ( struct line_buffer *linebuf, const char *data, size_t len ) { const char *eol; size_t consume; size_t new_len; char *new_data; - - /* Free any completed line from previous iteration */ - if ( linebuf->ready ) - empty_line_buffer ( linebuf ); + char *lf; + char *cr; /* Search for line terminator */ if ( ( eol = memchr ( data, '\n', len ) ) ) { @@ -94,6 +103,10 @@ ssize_t line_buffer ( struct line_buffer *linebuf, consume = len; } + /* Reject any embedded NULs within the data to be consumed */ + if ( memchr ( data, '\0', consume ) ) + return -EINVAL; + /* Reallocate data buffer and copy in new data */ new_len = ( linebuf->len + consume ); new_data = realloc ( linebuf->data, ( new_len + 1 ) ); @@ -104,13 +117,27 @@ ssize_t line_buffer ( struct line_buffer *linebuf, linebuf->data = new_data; linebuf->len = new_len; - /* If we have reached end of line, trim the line and mark as ready */ + /* If we have reached end of line, terminate the line */ if ( eol ) { - linebuf->data[--linebuf->len] = '\0'; /* trim NL */ - if ( linebuf->data[linebuf->len - 1] == '\r' ) - linebuf->data[--linebuf->len] = '\0'; /* trim CR */ - linebuf->ready = 1; + + /* Overwrite trailing LF (which must exist at this point) */ + assert ( linebuf->len > 0 ); + lf = &linebuf->data[ linebuf->len - 1 ]; + assert ( *lf == '\n' ); + *lf = '\0'; + + /* Trim (and overwrite) trailing CR, if present */ + if ( linebuf->len > 1 ) { + cr = ( lf - 1 ); + if ( *cr == '\r' ) { + linebuf->len--; + *cr = '\0'; + } + } } + /* Record consumed length */ + linebuf->consumed = consume; + return consume; } diff --git a/src/include/ipxe/linebuf.h b/src/include/ipxe/linebuf.h index a1d56bbf..630278a0 100644 --- a/src/include/ipxe/linebuf.h +++ b/src/include/ipxe/linebuf.h @@ -14,17 +14,17 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); /** A line buffer */ struct line_buffer { - /** Current string in the buffer */ + /** Data buffer */ char *data; - /** Length of current string, excluding the terminating NUL */ + /** Length of buffered data */ size_t len; - /** String is ready to read */ - int ready; + /** Most recently consumed length */ + size_t consumed; }; extern char * buffered_line ( struct line_buffer *linebuf ); -extern ssize_t line_buffer ( struct line_buffer *linebuf, - const char *data, size_t len ); +extern int line_buffer ( struct line_buffer *linebuf, + const char *data, size_t len ); extern void empty_line_buffer ( struct line_buffer *linebuf ); #endif /* _IPXE_LINEBUF_H */ diff --git a/src/tests/linebuf_test.c b/src/tests/linebuf_test.c index e06ac7d8..0dd486e9 100644 --- a/src/tests/linebuf_test.c +++ b/src/tests/linebuf_test.c @@ -1,35 +1,320 @@ -#include +/* + * Copyright (C) 2015 Michael Brown . + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + * + * You can also choose to distribute this program under the terms of + * the Unmodified Binary Distribution Licence (as given in the file + * COPYING.UBDL), provided that you have satisfied its requirements. + */ + +FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); + +/** @file + * + * Line buffer self-tests + * + */ + +/* Forcibly enable assertions */ +#undef NDEBUG + #include -#include +#include #include +#include -static const char data1[] = -"Hello world\r\n" -"This is a reasonably nice set of lines\n" -"with not many different terminators\r\n\r\n" -"There should be exactly one blank line above\n" -"and this line should never appear at all since it has no terminator"; +/** Define inline raw data */ +#define DATA(...) { __VA_ARGS__ } -void linebuf_test ( void ) { - struct line_buffer linebuf; - const char *data = data1; - size_t len = ( sizeof ( data1 ) - 1 /* be mean; strip the NUL */ ); - ssize_t frag_len; - char *line; +/** Define inline lines */ +#define LINES(...) { __VA_ARGS__ } - memset ( &linebuf, 0, sizeof ( linebuf ) ); - while ( len ) { - frag_len = line_buffer ( &linebuf, data, len ); - if ( frag_len < 0 ) { - printf ( "line_buffer() failed: %s\n", - strerror ( frag_len ) ); - return; - } - data += frag_len; - len -= frag_len; - if ( ( line = buffered_line ( &linebuf ) ) ) - printf ( "\"%s\"\n", line ); +/** A line buffer test */ +struct linebuf_test { + /** Raw data */ + const void *data; + /** Length of raw data */ + size_t len; + /** Expected sequence of lines */ + const char **lines; + /** Number of expected lines */ + unsigned int count; +}; + +/** Line buffer test expected failure indicator */ +static const char linebuf_failure[1]; + +/** + * Define a line buffer test + * + * @v name Test name + * @v DATA Raw data + * @v LINES Expected sequence of lines + * @ret test Line buffer test + */ +#define LINEBUF_TEST( name, DATA, LINES ) \ + static const char name ## _data[] = DATA; \ + static const char * name ## _lines[] = LINES; \ + static struct linebuf_test name = { \ + .data = name ## _data, \ + .len = ( sizeof ( name ## _data ) - 1 /* NUL */ ), \ + .lines = name ## _lines, \ + .count = ( sizeof ( name ## _lines ) / \ + sizeof ( name ## _lines[0] ) ), \ } - empty_line_buffer ( &linebuf ); +/** Simple line buffer test */ +LINEBUF_TEST ( simple, + ( "HTTP/1.1 200 OK\r\n" + "Content-Length: 123\r\n" + "Content-Type: text/plain\r\n" + "\r\n" ), + LINES ( "HTTP/1.1 200 OK", + "Content-Length: 123", + "Content-Type: text/plain", + "" ) ); + +/** Mixed line terminators */ +LINEBUF_TEST ( mixed, + ( "LF only\n" "CRLF\r\n" "\n" "\n" "\r\n" "\r\n" "CR only\r" ), + LINES ( "LF only", "CRLF", "", "", "", "", + NULL /* \r should not be treated as a terminator */ ) ); + +/** Split consumption: part 1 */ +LINEBUF_TEST ( split_1, + ( "This line was" ), + LINES ( NULL ) ); + +/** Split consumption: part 2 */ +LINEBUF_TEST ( split_2, + ( " split across" ), + LINES ( NULL ) ); + +/** Split consumption: part 3 */ +LINEBUF_TEST ( split_3, + ( " multiple calls\r\nand so was this one\r" ), + LINES ( "This line was split across multiple calls", NULL ) ); + +/** Split consumption: part 4 */ +LINEBUF_TEST ( split_4, + ( "\nbut not this one\r\n" ), + LINES ( "and so was this one", "but not this one" ) ); + +/** Split consumption: part 5 */ +LINEBUF_TEST ( split_5, + ( "" ), + LINES ( NULL ) ); + +/** Split consumption: part 6 */ +LINEBUF_TEST ( split_6, + ( "This line came after a zero-length call\r\n" ), + LINES ( "This line came after a zero-length call" ) ); + +/** Embedded NULs */ +LINEBUF_TEST ( embedded_nuls, + ( "This\r\ntest\r\nincludes\r\n\r\nsome\0binary\0data\r\n" ), + LINES ( "This", "test", "includes", "", linebuf_failure ) ); + +/** + * Report line buffer initialisation test result + * + * @v linebuf Line buffer + * @v file Test code file + * @v line Test code line + */ +static void linebuf_init_okx ( struct line_buffer *linebuf, + const char *file, unsigned int line ) { + + /* Initialise line buffer */ + memset ( linebuf, 0, sizeof ( *linebuf ) ); + okx ( buffered_line ( linebuf ) == NULL, file, line ); } +#define linebuf_init_ok( linebuf ) \ + linebuf_init_okx ( linebuf, __FILE__, __LINE__ ) + +/** + * Report line buffer consumption test result + * + * @v test Line buffer test + * @v linebuf Line buffer + * @v file Test code file + * @v line Test code line + */ +static void linebuf_consume_okx ( struct linebuf_test *test, + struct line_buffer *linebuf, + const char *file, unsigned int line ) { + const char *data = test->data; + size_t remaining = test->len; + int len; + unsigned int i; + const char *expected; + char *actual; + int rc; + + DBGC ( test, "LINEBUF %p:\n", test ); + DBGC_HDA ( test, 0, data, remaining ); + + /* Consume data one line at a time */ + for ( i = 0 ; i < test->count ; i++ ) { + + /* Add data to line buffer */ + len = line_buffer ( linebuf, data, remaining ); + + /* Get buffered line, if any */ + actual = buffered_line ( linebuf ); + if ( len < 0 ) { + rc = len; + DBGC ( test, "LINEBUF %p %s\n", test, strerror ( rc ) ); + } else if ( actual != NULL ) { + DBGC ( test, "LINEBUF %p \"%s\" (consumed %d)\n", + test, actual, len ); + } else { + DBGC ( test, "LINEBUF %p unterminated (consumed %d)\n", + test, len ); + } + + /* Check for success/failure */ + expected = test->lines[i]; + if ( expected == linebuf_failure ) { + rc = len; + okx ( rc < 0, file, line ); + okx ( remaining > 0, file, line ); + return; + } + okx ( len >= 0, file, line ); + okx ( ( ( size_t ) len ) <= remaining, file, line ); + + /* Check expected result */ + if ( expected == NULL ) { + okx ( actual == NULL, file, line ); + } else { + okx ( actual != NULL, file, line ); + okx ( strcmp ( actual, expected ) == 0, file, line ); + } + + /* Consume data */ + data += len; + remaining -= len; + } + + /* Check that all data was consumed */ + okx ( remaining == 0, file, line ); +} +#define linebuf_consume_ok( test, linebuf ) \ + linebuf_consume_okx ( test, linebuf, __FILE__, __LINE__ ) + +/** + * Report line buffer accumulation test result + * + * @v test Line buffer test + * @v linebuf Line buffer + * @v file Test code file + * @v line Test code line + */ +static void linebuf_accumulated_okx ( struct linebuf_test *test, + struct line_buffer *linebuf, + const char *file, unsigned int line ) { + const char *actual; + const char *expected; + unsigned int i; + + /* Check each accumulated line */ + actual = linebuf->data; + for ( i = 0 ; i < test->count ; i++ ) { + + /* Check accumulated line */ + okx ( actual != NULL, file, line ); + okx ( actual >= linebuf->data, file, line ); + expected = test->lines[i]; + if ( ( expected == NULL ) || ( expected == linebuf_failure ) ) + return; + okx ( strcmp ( actual, expected ) == 0, file, line ); + + /* Move to next line */ + actual += ( strlen ( actual ) + 1 /* NUL */ ); + okx ( actual <= ( linebuf->data + linebuf->len ), file, line ); + } +} +#define linebuf_accumulated_ok( test, linebuf ) \ + linebuf_accumulated_okx ( test, linebuf, __FILE__, __LINE__ ) + +/** + * Report line buffer emptying test result + * + * @v linebuf Line buffer + * @v file Test code file + * @v line Test code line + */ +static void linebuf_empty_okx ( struct line_buffer *linebuf, + const char *file, unsigned int line ) { + + /* Empty line buffer */ + empty_line_buffer ( linebuf ); + okx ( buffered_line ( linebuf ) == NULL, file, line ); +} +#define linebuf_empty_ok( linebuf ) \ + linebuf_empty_okx ( linebuf, __FILE__, __LINE__ ) + +/** + * Report line buffer combined test result + * + * @v test Line buffer test + * @v file Test code file + * @v line Test code line + */ +static void linebuf_okx ( struct linebuf_test *test, const char *file, + unsigned int line ) { + struct line_buffer linebuf; + + linebuf_init_okx ( &linebuf, file, line ); + linebuf_consume_okx ( test, &linebuf, file, line ); + linebuf_accumulated_okx ( test, &linebuf, file, line ); + linebuf_empty_okx ( &linebuf, file, line ); +} +#define linebuf_ok( test ) \ + linebuf_okx ( test, __FILE__, __LINE__ ) + +/** + * Perform line buffer self-tests + * + */ +static void linebuf_test_exec ( void ) { + struct line_buffer linebuf; + + /* Basic tests */ + linebuf_ok ( &simple ); + linebuf_ok ( &mixed ); + + /* Split consumption test */ + linebuf_init_ok ( &linebuf ); + linebuf_consume_ok ( &split_1, &linebuf ); + linebuf_consume_ok ( &split_2, &linebuf ); + linebuf_consume_ok ( &split_3, &linebuf ); + linebuf_consume_ok ( &split_4, &linebuf ); + linebuf_consume_ok ( &split_5, &linebuf ); + linebuf_consume_ok ( &split_6, &linebuf ); + linebuf_empty_ok ( &linebuf ); + + /* Embedded NULs */ + linebuf_ok ( &embedded_nuls ); +} + +/** Line buffer self-test */ +struct self_test linebuf_test __self_test = { + .name = "linebuf", + .exec = linebuf_test_exec, +}; diff --git a/src/tests/tests.c b/src/tests/tests.c index 0e6afee6..54ce8667 100644 --- a/src/tests/tests.c +++ b/src/tests/tests.c @@ -66,3 +66,4 @@ REQUIRE_OBJECT ( uri_test ); REQUIRE_OBJECT ( profile_test ); REQUIRE_OBJECT ( setjmp_test ); REQUIRE_OBJECT ( pccrc_test ); +REQUIRE_OBJECT ( linebuf_test );