diff --git a/Makefile b/Makefile index 5647da7..02118e9 100644 --- a/Makefile +++ b/Makefile @@ -30,10 +30,11 @@ $(BUILD_DIR)/test_pager: $(TEST_DIR)/test_pager.c $(SRC_DIR)/pager.c | $(BUILD_D $(BUILD_DIR)/test_parser: $(TEST_DIR)/test_parser.c $(SRC_DIR)/parser.c | $(BUILD_DIR) $(CC) $(CFLAGS) $(CPPFLAGS) $^ -o $@ -test: $(TEST_TARGET) $(BUILD_DIR)/test_pager $(BUILD_DIR)/test_parser +test: $(TARGET) $(TEST_TARGET) $(BUILD_DIR)/test_pager $(BUILD_DIR)/test_parser ./$(TEST_TARGET) ./$(BUILD_DIR)/test_pager ./$(BUILD_DIR)/test_parser + sh $(TEST_DIR)/test_shell.sh $(BUILD_DIR)/benchmark: bench/benchmark.c $(SRC_DIR)/database.c $(SRC_DIR)/btree.c $(SRC_DIR)/pager.c | $(BUILD_DIR) $(CC) $(CFLAGS) $(CPPFLAGS) $^ -o $@ diff --git a/README.md b/README.md index 53b1b98..3540eb0 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ The database is persisted to `build/sqlite.db`, so data can be written, the proc | Index structure | B tree with leaf pages and an internal root | | Persistence | Database metadata and B tree pages flushed to disk | | Shell | Interactive command line interface | -| Validation | Row ID, username, email, duplicate key checks | +| Validation | Row fields, duplicate keys, and persisted B tree structure | | Testing | Database, B tree, pager, and parser tests | | Benchmarking | 1,000 row insertion benchmark | | CI | Strict compiler warnings, benchmark smoke test, ASan and UBSan | @@ -104,7 +104,7 @@ The current storage design supports up to 100 pages. Internal node splitting bey ### Table API -`src/database.c` validates rows and exposes the public table operations used by the shell and tests. It also maintains the ordered in memory result view used by the shell. +`src/database.c` validates rows and exposes the public table operations used by the shell and tests. It maintains the ordered in memory result view used by the shell. Use `table_close` when the caller needs to detect persistence failures; `table_destroy` remains a compatibility wrapper. The persistent source of truth is the B tree. @@ -166,8 +166,11 @@ Ordered traversal Duplicate key rejection Point lookup Input validation +Malformed database rejection Pager behavior -SQL parser behavior +SQL parser overflow handling +Oversized shell command rejection +Checked close and persistence errors Run the full test suite: diff --git a/docs/architecture.md b/docs/architecture.md index 872d82f..23f88ef 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -59,8 +59,9 @@ Responsibilities include: - maintaining the ordered in-memory result view - exposing point lookup and row iteration - translating lower-level errors into public results +- reporting flush failures through `table_close` -The persistent source of truth is the B-tree. +The persistent source of truth is the B-tree. On open, the engine validates node types and capacities, page references, parent links, key ordering, separator values, leaf-chain order, and the metadata row count. Malformed database files are rejected before normal traversal begins. ## B-tree @@ -120,7 +121,7 @@ B-tree pages use a compact fixed layout. Rows are serialized directly into fixed ## Testing -The storage tests exercise persistence, ordering, B-tree leaf splitting, duplicate detection, point lookup, and validation. +The storage tests exercise persistence, ordering, B-tree leaf splitting, duplicate detection, point lookup, row validation, and rejection of malformed metadata and B-tree pages. Parser tests cover numeric overflow, and a shell integration test verifies that oversized commands are rejected without partial execution. The parser and pager have separate test binaries. CI additionally builds with strict warnings and runs the suite under AddressSanitizer and UndefinedBehaviorSanitizer. diff --git a/include/btree.h b/include/btree.h index 038d59f..3e02583 100644 --- a/include/btree.h +++ b/include/btree.h @@ -18,7 +18,7 @@ typedef enum { typedef struct BTree BTree; BTree *btree_open(Pager *pager, BTreeResult *result); -void btree_close(BTree *tree); +BTreeResult btree_close(BTree *tree); BTreeResult btree_flush(BTree *tree); BTreeResult btree_insert(BTree *tree, const Row *row); diff --git a/include/pager.h b/include/pager.h index d33823a..f8c9a39 100644 --- a/include/pager.h +++ b/include/pager.h @@ -22,7 +22,7 @@ typedef struct { } Pager; Pager *pager_open(const char *filename, PagerResult *result); -void pager_close(Pager *pager); +PagerResult pager_close(Pager *pager); void *pager_get_page(Pager *pager, size_t page_number, PagerResult *result); PagerResult pager_flush(Pager *pager, size_t page_number); diff --git a/include/sqlite.h b/include/sqlite.h index eaf0444..199f3f4 100644 --- a/include/sqlite.h +++ b/include/sqlite.h @@ -18,7 +18,8 @@ typedef enum { SQLITE_INVALID_EMAIL, SQLITE_DUPLICATE_ID, SQLITE_ROW_NOT_FOUND, - SQLITE_IO_ERROR + SQLITE_IO_ERROR, + SQLITE_CORRUPT } SqliteResult; typedef struct { @@ -31,6 +32,7 @@ typedef struct Table Table; Table *table_open(const char *filename, SqliteResult *result); void table_destroy(Table *table); +SqliteResult table_close(Table *table); SqliteResult table_flush(Table *table); void table_print_tree(const Table *table); diff --git a/src/btree.c b/src/btree.c index 0d63120..bec39d2 100644 --- a/src/btree.c +++ b/src/btree.c @@ -202,18 +202,19 @@ static uint32_t allocate_page(BTree *tree, BTreeResult *result) return page_number; } -static void write_metadata(BTree *tree) +static BTreeResult write_metadata(BTree *tree) { BTreeResult result; unsigned char *page = page_for(tree, 0, &result); if (page == NULL) { - return; + return result; } write_u32(page, META_MAGIC); write_u32(page + 4U, META_VERSION); write_u32(page + 8U, tree->root_page); write_u32(page + 12U, tree->row_count); + return BTREE_OK; } static uint32_t leaf_find_index(const unsigned char *page, int id) @@ -480,6 +481,131 @@ static BTreeResult split_leaf(BTree *tree, uint32_t leaf_page, return BTREE_OK; } + +static int validate_node(BTree *tree, uint32_t page_number, uint32_t expected_parent, + unsigned char *visited, uint32_t *leaf_pages, + size_t *leaf_count, uint32_t *row_count, + uint32_t *minimum, uint32_t *maximum, int is_root) +{ + if (page_number == 0U || page_number >= tree->pager->num_pages || + page_number >= TABLE_MAX_PAGES || visited[page_number] != 0U) { + return 0; + } + + visited[page_number] = 1U; + + BTreeResult result; + const unsigned char *page = const_page_for(tree, page_number, &result); + if (page == NULL || result != BTREE_OK || node_parent(page) != expected_parent) { + return 0; + } + + uint8_t type = node_type(page); + uint32_t count = node_count(page); + + if (type == NODE_LEAF) { + if (count > LEAF_MAX_CELLS || (!is_root && count == 0U)) { + return 0; + } + + uint32_t previous = 0U; + for (uint32_t i = 0; i < count; ++i) { + uint32_t key = leaf_key(page, i); + if (key == 0U || key > (uint32_t)INT32_MAX || + (i > 0U && key <= previous)) { + return 0; + } + previous = key; + } + + uint32_t next = leaf_next(page); + if (next >= tree->pager->num_pages && next != 0U) { + return 0; + } + + leaf_pages[*leaf_count] = page_number; + ++(*leaf_count); + *row_count += count; + *minimum = count == 0U ? 0U : leaf_key(page, 0U); + *maximum = count == 0U ? 0U : leaf_key(page, count - 1U); + return 1; + } + + if (type != NODE_INTERNAL || !is_root || count == 0U || + count > INTERNAL_MAX_CELLS) { + return 0; + } + + uint32_t previous_maximum = 0U; + for (uint32_t i = 0; i <= count; ++i) { + uint32_t child = child_page_at(page, i); + uint32_t child_minimum = 0U; + uint32_t child_maximum = 0U; + if (!validate_node(tree, child, page_number, visited, leaf_pages, + leaf_count, row_count, &child_minimum, + &child_maximum, 0)) { + return 0; + } + + if (child_minimum == 0U || child_maximum == 0U || + (i > 0U && child_minimum <= previous_maximum)) { + return 0; + } + + if (i < count && internal_key(page, i) != child_maximum) { + return 0; + } + + if (i == 0U) { + *minimum = child_minimum; + } + previous_maximum = child_maximum; + } + + *maximum = previous_maximum; + return 1; +} + +static int validate_tree(BTree *tree) +{ + unsigned char visited[TABLE_MAX_PAGES] = {0}; + uint32_t leaf_pages[TABLE_MAX_PAGES] = {0}; + size_t leaf_count = 0U; + uint32_t row_count = 0U; + uint32_t minimum = 0U; + uint32_t maximum = 0U; + + if (!validate_node(tree, tree->root_page, 0U, visited, leaf_pages, + &leaf_count, &row_count, &minimum, &maximum, 1)) { + return 0; + } + + if (row_count != tree->row_count || leaf_count == 0U) { + return 0; + } + + for (size_t page_number = 1U; page_number < tree->pager->num_pages; ++page_number) { + if (visited[page_number] == 0U) { + return 0; + } + } + + for (size_t i = 0; i < leaf_count; ++i) { + BTreeResult result; + const unsigned char *leaf = const_page_for(tree, leaf_pages[i], &result); + if (leaf == NULL || result != BTREE_OK) { + return 0; + } + + uint32_t expected_next = i + 1U < leaf_count ? leaf_pages[i + 1U] : 0U; + if (leaf_next(leaf) != expected_next) { + return 0; + } + } + + return 1; +} + BTree *btree_open(Pager *pager, BTreeResult *result) { if (pager == NULL || result == NULL) { @@ -504,7 +630,7 @@ BTree *btree_open(Pager *pager, BTreeResult *result) return NULL; } - if (pager->num_pages == 1U && read_u32(meta) == 0U) { + if (pager->file_length == 0U) { tree->root_page = 1; tree->row_count = 0; @@ -515,7 +641,11 @@ BTree *btree_open(Pager *pager, BTreeResult *result) } initialize_leaf(root, 0); - write_metadata(tree); + if (write_metadata(tree) != BTREE_OK) { + free(tree); + *result = BTREE_IO_ERROR; + return NULL; + } return tree; } @@ -535,6 +665,12 @@ BTree *btree_open(Pager *pager, BTreeResult *result) return NULL; } + if (!validate_tree(tree)) { + free(tree); + *result = BTREE_CORRUPT; + return NULL; + } + return tree; } @@ -544,7 +680,10 @@ BTreeResult btree_flush(BTree *tree) return BTREE_INVALID_ARGUMENT; } - write_metadata(tree); + BTreeResult metadata_result = write_metadata(tree); + if (metadata_result != BTREE_OK) { + return metadata_result; + } for (size_t page_number = 0; page_number < tree->pager->num_pages; ++page_number) { if (tree->pager->pages[page_number] != NULL && @@ -556,14 +695,15 @@ BTreeResult btree_flush(BTree *tree) return BTREE_OK; } -void btree_close(BTree *tree) +BTreeResult btree_close(BTree *tree) { if (tree == NULL) { - return; + return BTREE_INVALID_ARGUMENT; } - (void)btree_flush(tree); + BTreeResult result = btree_flush(tree); free(tree); + return result; } BTreeResult btree_insert(BTree *tree, const Row *row) diff --git a/src/database.c b/src/database.c index 72208e7..afc946d 100644 --- a/src/database.c +++ b/src/database.c @@ -72,7 +72,7 @@ Table *table_open(const char *filename, SqliteResult *result) pager_close(pager); free(table->rows); free(table); - *result = SQLITE_IO_ERROR; + *result = tree_result == BTREE_CORRUPT ? SQLITE_CORRUPT : SQLITE_IO_ERROR; return NULL; } @@ -106,22 +106,33 @@ void table_print_tree(const Table *table) } } -void table_destroy(Table *table) +SqliteResult table_close(Table *table) { if (table == NULL) { - return; + return SQLITE_IO_ERROR; } - if (table->tree != NULL) { - btree_close(table->tree); + SqliteResult result = SQLITE_OK; + if (table->tree != NULL && btree_close(table->tree) != BTREE_OK) { + result = SQLITE_IO_ERROR; } + table->tree = NULL; - if (table->pager != NULL) { - pager_close(table->pager); + if (table->pager != NULL && pager_close(table->pager) != PAGER_OK) { + result = SQLITE_IO_ERROR; } + table->pager = NULL; free(table->rows); free(table); + return result; +} + +void table_destroy(Table *table) +{ + if (table != NULL) { + (void)table_close(table); + } } SqliteResult table_insert(Table *table, const Row *row) @@ -154,8 +165,9 @@ SqliteResult table_insert(Table *table, const Row *row) case BTREE_TABLE_FULL: return SQLITE_TABLE_FULL; case BTREE_IO_ERROR: - case BTREE_CORRUPT: return SQLITE_IO_ERROR; + case BTREE_CORRUPT: + return SQLITE_CORRUPT; default: return SQLITE_INVALID_ID; } @@ -226,6 +238,8 @@ const char *sqlite_result_string(SqliteResult result) return "row not found"; case SQLITE_IO_ERROR: return "database I/O error"; + case SQLITE_CORRUPT: + return "database file is corrupt"; default: return "unknown error"; } diff --git a/src/main.c b/src/main.c index 8550bee..b0c2a23 100644 --- a/src/main.c +++ b/src/main.c @@ -51,6 +51,14 @@ int main(void) break; } + if (strchr(input, '\n') == NULL && !feof(stdin)) { + int ch; + while ((ch = getchar()) != '\n' && ch != EOF) { + } + puts("command too long"); + continue; + } + input[strcspn(input, "\n")] = '\0'; Statement statement; @@ -94,12 +102,10 @@ int main(void) } done: - if (table_flush(table) != SQLITE_OK) { + if (table_close(table) != SQLITE_OK) { fputs("failed to flush database\n", stderr); - table_destroy(table); return 1; } - table_destroy(table); return 0; } diff --git a/src/pager.c b/src/pager.c index 6b3f7c5..0e9c4f3 100644 --- a/src/pager.c +++ b/src/pager.c @@ -59,21 +59,27 @@ Pager *pager_open(const char *filename, PagerResult *result) return pager; } -void pager_close(Pager *pager) +PagerResult pager_close(Pager *pager) { if (pager == NULL) { - return; + return PAGER_INVALID_ARGUMENT; } + PagerResult result = PAGER_OK; for (size_t page_number = 0; page_number < pager->num_pages; ++page_number) { if (pager->pages[page_number] != NULL) { - (void)pager_flush(pager, page_number); + if (pager_flush(pager, page_number) != PAGER_OK) { + result = PAGER_IO_ERROR; + } free(pager->pages[page_number]); } } - fclose(pager->file); + if (fclose(pager->file) != 0) { + result = PAGER_IO_ERROR; + } free(pager); + return result; } void *pager_get_page(Pager *pager, size_t page_number, PagerResult *result) diff --git a/src/parser.c b/src/parser.c index 9ff65e4..3bd95e8 100644 --- a/src/parser.c +++ b/src/parser.c @@ -1,9 +1,25 @@ #include "parser.h" #include +#include +#include #include +#include #include +static int parse_positive_id(const char *text, int *id) +{ + char *end = NULL; + errno = 0; + long value = strtol(text, &end, 10); + if (errno == ERANGE || end == text || *end != '\0' || + value <= 0L || value > INT_MAX) { + return 0; + } + *id = (int)value; + return 1; +} + static int valid_suffix(const char *suffix) { while (isspace((unsigned char)*suffix)) { @@ -24,10 +40,11 @@ static int valid_suffix(const char *suffix) static int parse_insert(const char *input, Statement *statement) { int consumed = 0; + char id_text[11] = {0}; int matched = sscanf( input, - "INSERT INTO users VALUES (%d, '%32[^']', '%255[^']')%n", - &statement->row.id, + "INSERT INTO users VALUES ( %10[0-9] , '%32[^']', '%255[^']')%n", + id_text, statement->row.username, statement->row.email, &consumed @@ -35,10 +52,11 @@ static int parse_insert(const char *input, Statement *statement) if (matched != 3) { consumed = 0; + id_text[0] = '\0'; matched = sscanf( input, - "insert into users values (%d, '%32[^']', '%255[^']')%n", - &statement->row.id, + "insert into users values ( %10[0-9] , '%32[^']', '%255[^']')%n", + id_text, statement->row.username, statement->row.email, &consumed @@ -46,7 +64,7 @@ static int parse_insert(const char *input, Statement *statement) } if (matched == 3 && consumed > 0 && valid_suffix(input + consumed) && - statement->row.id > 0) { + parse_positive_id(id_text, &statement->row.id)) { statement->type = STATEMENT_INSERT; return 1; } @@ -57,17 +75,18 @@ static int parse_insert(const char *input, Statement *statement) static int parse_legacy_insert(const char *input, Statement *statement) { int consumed = 0; + char id_text[11] = {0}; int matched = sscanf( input, - "insert %d %32s %255s%n", - &statement->row.id, + "insert %10[0-9] %32s %255s%n", + id_text, statement->row.username, statement->row.email, &consumed ); if (matched == 3 && consumed > 0 && valid_suffix(input + consumed) && - statement->row.id > 0) { + parse_positive_id(id_text, &statement->row.id)) { statement->type = STATEMENT_INSERT; return 1; } @@ -93,17 +112,19 @@ static int parse_select(const char *input, Statement *statement) } int consumed = 0; - int matched = sscanf(input, "SELECT * FROM users WHERE id = %d%n", - &statement->where_id, &consumed); + char id_text[11] = {0}; + int matched = sscanf(input, "SELECT * FROM users WHERE id = %10[0-9]%n", + id_text, &consumed); if (matched != 1 || consumed <= 0 || !valid_suffix(input + consumed)) { consumed = 0; - matched = sscanf(input, "select * from users where id = %d%n", - &statement->where_id, &consumed); + id_text[0] = '\0'; + matched = sscanf(input, "select * from users where id = %10[0-9]%n", + id_text, &consumed); } if (matched == 1 && consumed > 0 && valid_suffix(input + consumed) && - statement->where_id > 0) { + parse_positive_id(id_text, &statement->where_id)) { statement->type = STATEMENT_SELECT; return 1; } diff --git a/tests/test_database.c b/tests/test_database.c index ce9f448..db152b3 100644 --- a/tests/test_database.c +++ b/tests/test_database.c @@ -30,7 +30,7 @@ static void test_insert_and_read(void) assert(table_insert(table, &second) == SQLITE_OK); assert(table_size(table) == 2); - table_destroy(table); + assert(table_close(table) == SQLITE_OK); table = table_open(filename, &result); assert(table != NULL); @@ -42,7 +42,7 @@ static void test_insert_and_read(void) assert(strcmp(row->username, "user2") == 0); assert(strcmp(row->email, "user2@example.com") == 0); - table_destroy(table); + assert(table_close(table) == SQLITE_OK); remove(filename); } @@ -181,6 +181,87 @@ static void test_validation(void) remove(filename); } + +static void write_u32_le(FILE *file, long offset, unsigned int value) +{ + unsigned char bytes[4] = { + (unsigned char)(value & 0xffU), + (unsigned char)((value >> 8U) & 0xffU), + (unsigned char)((value >> 16U) & 0xffU), + (unsigned char)((value >> 24U) & 0xffU) + }; + + assert(fseek(file, offset, SEEK_SET) == 0); + assert(fwrite(bytes, sizeof(bytes), 1, file) == 1); + assert(fclose(file) == 0); +} + +static void create_database_with_split(const char *filename) +{ + remove(filename); + SqliteResult result; + Table *table = table_open(filename, &result); + assert(table != NULL); + + for (int id = 1; id <= 20; ++id) { + Row row = make_row(id); + assert(table_insert(table, &row) == SQLITE_OK); + } + + assert(table_flush(table) == SQLITE_OK); + table_destroy(table); +} + +static void test_corrupt_node_count_is_rejected(void) +{ + const char *filename = "build/test_corrupt_count.db"; + create_database_with_split(filename); + + FILE *file = fopen(filename, "r+b"); + assert(file != NULL); + write_u32_le(file, (long)PAGE_SIZE + 8L, 0xffffffffU); + + SqliteResult result; + Table *table = table_open(filename, &result); + assert(table == NULL); + assert(result == SQLITE_CORRUPT); + remove(filename); +} + +static void test_corrupt_metadata_page_is_rejected(void) +{ + const char *filename = "build/test_corrupt_metadata.db"; + remove(filename); + + FILE *file = fopen(filename, "wb"); + assert(file != NULL); + unsigned char page[PAGE_SIZE] = {0}; + assert(fwrite(page, sizeof(page), 1, file) == 1); + assert(fclose(file) == 0); + + SqliteResult result; + Table *table = table_open(filename, &result); + assert(table == NULL); + assert(result == SQLITE_CORRUPT); + remove(filename); +} + +static void test_corrupt_child_reference_is_rejected(void) +{ + const char *filename = "build/test_corrupt_child.db"; + create_database_with_split(filename); + + FILE *file = fopen(filename, "r+b"); + assert(file != NULL); + write_u32_le(file, (long)PAGE_SIZE + 16L, 0U); + + SqliteResult result; + Table *table = table_open(filename, &result); + assert(table == NULL); + assert(result == SQLITE_CORRUPT); + remove(filename); +} + int main(void) { test_insert_and_read(); @@ -188,5 +269,8 @@ int main(void) test_btree_mixed_insertion_order(); test_lookup(); test_validation(); + test_corrupt_node_count_is_rejected(); + test_corrupt_metadata_page_is_rejected(); + test_corrupt_child_reference_is_rejected(); return 0; } diff --git a/tests/test_parser.c b/tests/test_parser.c index 8825bb4..d821580 100644 --- a/tests/test_parser.c +++ b/tests/test_parser.c @@ -60,6 +60,16 @@ static void test_meta_command(void) assert(statement.type == META_BTREE); } +static void test_numeric_overflow_is_rejected(void) +{ + Statement statement; + assert(parse_statement( + "INSERT INTO users VALUES (2147483648, 'alice', 'alice@example.com');", + &statement) == PARSER_SYNTAX_ERROR); + assert(parse_statement("SELECT * FROM users WHERE id = 999999999999999999999;", + &statement) == PARSER_SYNTAX_ERROR); +} + static void test_invalid_input(void) { Statement statement; @@ -77,5 +87,6 @@ int main(void) test_select_allows_trailing_whitespace(); test_meta_command(); test_invalid_input(); + test_numeric_overflow_is_rejected(); return 0; } diff --git a/tests/test_shell.sh b/tests/test_shell.sh new file mode 100644 index 0000000..b964f5d --- /dev/null +++ b/tests/test_shell.sh @@ -0,0 +1,25 @@ +#!/bin/sh +set -eu + +database="build/sqlite.db" +rm -f "$database" + +output=$( + { + printf "INSERT INTO users VALUES (1, 'alice', 'alice@example.com');" + printf '%600s' '' + printf '\nSELECT * FROM users;\n.exit\n' + } | ./build/sqlite-c +) + +printf '%s\n' "$output" | grep -q "command too long" +if printf '%s\n' "$output" | grep -q "1 | alice | alice@example.com"; then + echo "oversized command was partially executed" >&2 + exit 1 +fi + +rm -f "$database" +output=$(printf "INSERT INTO users VALUES (1, 'alice', 'alice@example.com');\nSELECT * FROM users;\n.exit\n" | ./build/sqlite-c) +printf '%s\n' "$output" | grep -q "1 | alice | alice@example.com" + +rm -f "$database"