From ee5d14c4cd7cee0e852fcbb882c7c9914dd7573d Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Thu, 14 Aug 2025 16:49:20 -0400 Subject: [PATCH] Clear key after matching on it --- src/commit_request.cpp | 76 ++++++++++++++++++++++++----------- src/commit_request.hpp | 12 +++++- tests/test_commit_request.cpp | 7 +++- 3 files changed, 67 insertions(+), 28 deletions(-) diff --git a/src/commit_request.cpp b/src/commit_request.cpp index 539aa07..f3ffcc6 100644 --- a/src/commit_request.cpp +++ b/src/commit_request.cpp @@ -116,7 +116,7 @@ void CommitRequest::on_begin_object(void *userdata) { return; if (ctx.state_stack.empty()) { - ctx.parse_error = true; + ctx.parse_error = "Unexpected object start - empty state stack"; return; } @@ -135,7 +135,7 @@ void CommitRequest::on_begin_object(void *userdata) { ctx.current_operation = OperationParseState{}; break; default: - ctx.parse_error = true; + ctx.parse_error = "Unexpected object in invalid parse state"; break; } } @@ -145,7 +145,9 @@ void CommitRequest::on_end_object(void *userdata) { auto &ctx = self->parser_context_; if (ctx.parse_error || ctx.state_stack.empty()) { - ctx.parse_error = true; + if (!ctx.parse_error) { + ctx.parse_error = "Unexpected object end - empty state stack"; + } return; } @@ -162,7 +164,8 @@ void CommitRequest::on_end_object(void *userdata) { switch (ctx.current_precondition.type) { case Precondition::Type::PointRead: if (!ctx.current_precondition.key.has_value()) { - ctx.parse_error = true; + ctx.parse_error = + "point_read precondition missing required 'key' field"; } else { self->preconditions_.push_back( Precondition{ctx.current_precondition.type, @@ -174,7 +177,8 @@ void CommitRequest::on_end_object(void *userdata) { case Precondition::Type::RangeRead: if (!ctx.current_precondition.begin.has_value() || !ctx.current_precondition.end.has_value()) { - ctx.parse_error = true; + ctx.parse_error = "range_read precondition missing required 'begin' " + "and/or 'end' fields"; } else { self->preconditions_.push_back( Precondition{ctx.current_precondition.type, @@ -190,7 +194,8 @@ void CommitRequest::on_end_object(void *userdata) { case Operation::Type::Write: if (!ctx.current_operation.key.has_value() || !ctx.current_operation.value.has_value()) { - ctx.parse_error = true; + ctx.parse_error = + "write operation missing required 'key' and/or 'value' fields"; } else { self->operations_.push_back(Operation{ ctx.current_operation.type, ctx.current_operation.key.value(), @@ -199,7 +204,7 @@ void CommitRequest::on_end_object(void *userdata) { break; case Operation::Type::Delete: if (!ctx.current_operation.key.has_value()) { - ctx.parse_error = true; + ctx.parse_error = "delete operation missing required 'key' field"; } else { self->operations_.push_back(Operation{ ctx.current_operation.type, ctx.current_operation.key.value(), {}}); @@ -208,7 +213,8 @@ void CommitRequest::on_end_object(void *userdata) { case Operation::Type::RangeDelete: if (!ctx.current_operation.begin.has_value() || !ctx.current_operation.end.has_value()) { - ctx.parse_error = true; + ctx.parse_error = "range_delete operation missing required 'begin' " + "and/or 'end' fields"; } else { self->operations_.push_back(Operation{ ctx.current_operation.type, ctx.current_operation.begin.value(), @@ -235,7 +241,6 @@ void CommitRequest::on_string_data(void *userdata, const char *buf, int len, if (done) { self->handle_completed_string(); ctx.current_string.clear(); - ctx.current_key.clear(); // Clear key after processing value } } @@ -263,11 +268,14 @@ void CommitRequest::on_begin_array(void *userdata) { return; if (ctx.current_key == "preconditions") { + ctx.current_key.clear(); ctx.state_stack.push(ParseState::PreconditionsArray); } else if (ctx.current_key == "operations") { + ctx.current_key.clear(); ctx.state_stack.push(ParseState::OperationsArray); } else { - ctx.parse_error = true; + ctx.parse_error = "Invalid array field - only 'preconditions' and " + "'operations' arrays are allowed"; } } @@ -276,7 +284,9 @@ void CommitRequest::on_end_array(void *userdata) { auto &ctx = self->parser_context_; if (ctx.parse_error || ctx.state_stack.empty()) { - ctx.parse_error = true; + if (!ctx.parse_error) { + ctx.parse_error = "Unexpected array end - empty state stack"; + } return; } @@ -296,19 +306,18 @@ void CommitRequest::on_number_data(void *userdata, const char *buf, int len, if (done) { self->handle_completed_number(); ctx.current_number.clear(); - ctx.current_key.clear(); // Clear key after processing value } } -void CommitRequest::on_true_literal(void *userdata) { +void CommitRequest::on_true_literal(void *) { // Not used in this API } -void CommitRequest::on_false_literal(void *userdata) { +void CommitRequest::on_false_literal(void *) { // Not used in this API } -void CommitRequest::on_null_literal(void *userdata) { +void CommitRequest::on_null_literal(void *) { // Not used in this API } @@ -316,7 +325,7 @@ void CommitRequest::handle_completed_string() { auto &ctx = parser_context_; if (ctx.state_stack.empty()) { - ctx.parse_error = true; + ctx.parse_error = "String value received with empty state stack"; return; } @@ -325,33 +334,42 @@ void CommitRequest::handle_completed_string() { switch (current_state) { case ParseState::Root: if (ctx.current_key == "request_id") { + ctx.current_key.clear(); request_id_ = store_string(ctx.current_string); } else if (ctx.current_key == "leader_id") { + ctx.current_key.clear(); leader_id_ = store_string(ctx.current_string); } else if (ctx.current_key == "read_version") { + ctx.current_key.clear(); // read_version should be a number, not a string - ctx.parse_error = true; + ctx.parse_error = "read_version field must be a number, not a string"; } break; case ParseState::PreconditionObject: if (ctx.current_key == "type") { + ctx.current_key.clear(); if (ctx.current_string == "point_read") { ctx.current_precondition.type = Precondition::Type::PointRead; } else if (ctx.current_string == "range_read") { ctx.current_precondition.type = Precondition::Type::RangeRead; } else { - ctx.parse_error = true; + ctx.parse_error = + "Invalid precondition type - must be 'point_read' or 'range_read'"; } } else if (ctx.current_key == "key") { + ctx.current_key.clear(); ctx.current_precondition.key = decode_base64(ctx.current_string); } else if (ctx.current_key == "begin") { + ctx.current_key.clear(); ctx.current_precondition.begin = decode_base64(ctx.current_string); } else if (ctx.current_key == "end") { + ctx.current_key.clear(); ctx.current_precondition.end = decode_base64(ctx.current_string); } break; case ParseState::OperationObject: if (ctx.current_key == "type") { + ctx.current_key.clear(); if (ctx.current_string == "write") { ctx.current_operation.type = Operation::Type::Write; } else if (ctx.current_string == "delete") { @@ -359,15 +377,20 @@ void CommitRequest::handle_completed_string() { } else if (ctx.current_string == "range_delete") { ctx.current_operation.type = Operation::Type::RangeDelete; } else { - ctx.parse_error = true; + ctx.parse_error = "Invalid operation type - must be 'write', 'delete', " + "or 'range_delete'"; } } else if (ctx.current_key == "key") { + ctx.current_key.clear(); ctx.current_operation.key = decode_base64(ctx.current_string); } else if (ctx.current_key == "value") { + ctx.current_key.clear(); ctx.current_operation.value = decode_base64(ctx.current_string); } else if (ctx.current_key == "begin") { + ctx.current_key.clear(); ctx.current_operation.begin = decode_base64(ctx.current_string); } else if (ctx.current_key == "end") { + ctx.current_key.clear(); ctx.current_operation.end = decode_base64(ctx.current_string); } break; @@ -380,7 +403,7 @@ void CommitRequest::handle_completed_number() { auto &ctx = parser_context_; if (ctx.state_stack.empty()) { - ctx.parse_error = true; + ctx.parse_error = "Number value received with empty state stack"; return; } @@ -389,6 +412,7 @@ void CommitRequest::handle_completed_number() { switch (current_state) { case ParseState::Root: if (ctx.current_key == "read_version") { + ctx.current_key.clear(); uint64_t version; auto result = std::from_chars( ctx.current_number.data(), @@ -397,12 +421,13 @@ void CommitRequest::handle_completed_number() { read_version_ = version; has_read_version_been_set_ = true; } else { - ctx.parse_error = true; + ctx.parse_error = "Invalid number format for read_version field"; } } break; case ParseState::PreconditionObject: if (ctx.current_key == "version") { + ctx.current_key.clear(); uint64_t version; auto result = std::from_chars( ctx.current_number.data(), @@ -410,7 +435,8 @@ void CommitRequest::handle_completed_number() { if (result.ec == std::errc{}) { ctx.current_precondition.version = version; } else { - ctx.parse_error = true; + ctx.parse_error = + "Invalid number format for precondition version field"; } } break; @@ -462,7 +488,8 @@ CommitRequest::ParseStatus CommitRequest::parse_chunk(char *data, size_t len) { case WeaselJson_REJECT: case WeaselJson_OVERFLOW: default: - parser_context_.parse_error = true; + parser_context_.parse_error = + "JSON parsing failed - invalid or oversized JSON"; return ParseStatus::Error; } } @@ -483,7 +510,8 @@ CommitRequest::ParseStatus CommitRequest::finish_streaming_parse() { !parser_context_.parse_error) { return ParseStatus::Complete; } else { - parser_context_.parse_error = true; + parser_context_.parse_error = + "JSON parsing incomplete or failed during finalization"; return ParseStatus::Error; } } diff --git a/src/commit_request.hpp b/src/commit_request.hpp index d24b36d..7ddb44c 100644 --- a/src/commit_request.hpp +++ b/src/commit_request.hpp @@ -89,7 +89,7 @@ public: ArenaString current_string; ArenaString current_number; bool in_key = false; - bool parse_error = false; + const char *parse_error = nullptr; bool parse_complete = false; // Current objects being parsed @@ -222,7 +222,15 @@ public: * @brief Check if there was a parse error. * @return true if there was a parse error */ - bool has_parse_error() const { return parser_context_.parse_error; } + bool has_parse_error() const { + return parser_context_.parse_error != nullptr; + } + + /** + * @brief Get the parse error message if there was an error. + * @return Error message string, or nullptr if no error + */ + const char *get_parse_error() const { return parser_context_.parse_error; } /** * @brief Get the request ID if present. diff --git a/tests/test_commit_request.cpp b/tests/test_commit_request.cpp index 4c396d2..dc5ca31 100644 --- a/tests/test_commit_request.cpp +++ b/tests/test_commit_request.cpp @@ -195,8 +195,8 @@ TEST_CASE("CommitRequest precondition and operation validation") { { "type": "range_read", "version": 12340, - "begin": "", - "end": "" + "begin": "dGVzdA==", + "end": "dGVzdFo=" } ] })"; @@ -206,6 +206,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { INFO("Parse result: " << parse_result); INFO("Parse complete: " << request.is_parse_complete()); INFO("Parse error: " << request.has_parse_error()); + const char *error_msg = request.get_parse_error(); + INFO("Parse error message: " << (error_msg ? std::string(error_msg) + : "none")); INFO("Leader ID: '" << request.leader_id() << "'"); INFO("Read version: " << request.read_version());