From 64f0f6a13e8581e175cc5044eadf520cbba66ada Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 18 May 2017 18:57:28 -0400 Subject: [PATCH 09/31] FIXME: better json error handling; WIP on server --- gcc/http-server.c | 7 +- gcc/http-server.h | 5 ++ gcc/json-rpc.c | 66 ++++++++++++--- gcc/json-rpc.h | 2 +- gcc/json.c | 170 ++++++++++++++++++++++++++++----------- gcc/json.h | 5 +- gcc/lsp-main.c | 4 +- gcc/testsuite/gcc.dg/lsp/test.py | 30 +++++++ 8 files changed, 227 insertions(+), 62 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/lsp/test.py diff --git a/gcc/http-server.c b/gcc/http-server.c index 1e1e8c8..3f7f7cd 100644 --- a/gcc/http-server.c +++ b/gcc/http-server.c @@ -155,7 +155,8 @@ server::read_from_client (int filedes) void httpish_server::on_read (size_t sz, const char *buf) { - fprintf (stderr, "httpish_server::on_read: `%s'\n", buf); // FIXME: respect sz + if (m_verbose) + fprintf (stderr, "httpish_server::on_read: `%s'\n", buf); // FIXME: respect sz size_t line_start = 0; while (line_start < sz) { @@ -204,7 +205,9 @@ namespace { class test_server : public httpish_server { public: - test_server () : m_user_agent (NULL), m_content (NULL) {} + test_server () + : httpish_server (false), m_user_agent (NULL), m_content (NULL) {} + ~test_server () { free (m_user_agent); free (m_content); } void on_header (size_t key_sz, const char *key, diff --git a/gcc/http-server.h b/gcc/http-server.h index 233f194..08d27b0 100644 --- a/gcc/http-server.h +++ b/gcc/http-server.h @@ -41,6 +41,8 @@ class server class httpish_server : public server { public: + httpish_server (bool verbose) : m_verbose (verbose) {} + void on_read (size_t sz, const char *buf) OVERRIDE FINAL; virtual void on_header (size_t key_sz, const char *key, @@ -49,6 +51,9 @@ class httpish_server : public server private: void parse_header (size_t sz, const char *buf); + + private: + bool m_verbose; }; #endif /* GCC_HTTP_SERVER_H */ diff --git a/gcc/json-rpc.c b/gcc/json-rpc.c index c92b2bb..a9bafbd 100644 --- a/gcc/json-rpc.c +++ b/gcc/json-rpc.c @@ -47,9 +47,9 @@ jsonrpc::make_failure (int code, const char *message, const json::value *id) } json::value * -jsonrpc::make_parse_error () +jsonrpc::make_parse_error (const char *msg) { - return make_failure (PARSE_ERROR, "parse error", NULL); + return make_failure (PARSE_ERROR, msg, NULL); } json::value * @@ -111,7 +111,7 @@ jsonrpc::server::handle_request_1 (json::value *request) { // FIXME: handle batch requests if (request->get_kind () != json::JSON_OBJECT) - return make_parse_error (); + return make_parse_error ("not an object"); json::object *reqobj = static_cast (request); @@ -146,7 +146,16 @@ jsonrpc::server::handle_request_1 (json::value *request) json::value * jsonrpc::server::handle_request (size_t sz, const char *buf) { - json::value *request = json::parse_utf8_string (sz, buf); + char *err = NULL; + json::value *request = json::parse_utf8_string (sz, buf, &err); + if (!request) + { + gcc_assert (err); + json::value *response = make_parse_error (err); + free (err); + return response; + } + gcc_assert (err == NULL); json::value *response = handle_request (request); delete request; return response; @@ -155,10 +164,7 @@ jsonrpc::server::handle_request (size_t sz, const char *buf) json::value * jsonrpc::server::handle_request_string (const char *utf8) { - json::value *request = json::parse_utf8_string (utf8); - json::value *response = handle_request (request); - delete request; - return response; + return handle_request (strlen (utf8), utf8); } #if CHECKING_P @@ -184,6 +190,22 @@ assert_is_response (const json::value *response, int id) return obj; } +/* Assert that RESPONSE is a non-NULL response, with no ID. + Verify that the jsonrpc value is "2.0". + Return RESPONSE, cast to an object *. */ + +static const json::object * +assert_is_response (const json::value *response) +{ + ASSERT_TRUE (response != NULL); + const json::object *obj = response->as_object (); + ASSERT_TRUE (obj != NULL); + json::value *jsonrpc = obj->get ("jsonrpc"); + ASSERT_TRUE (jsonrpc != NULL); + ASSERT_STREQ ("2.0", jsonrpc->as_string ()->get_string ()); + return obj; +} + /* Assert that RESPONSE is a non-NULL successful response for ID. Verify that the jsonrpc value is "2.0". Return the "result" value. */ @@ -211,6 +233,20 @@ assert_is_failure (const json::value *response, int code, const char *message, ASSERT_STREQ (message, err->get ("message")->as_string ()->get_string ()); } +/* Assert that RESPONSE is a non-NULL failure response, with no ID. + Verify that the jsonrpc value is "2.0". + Verify that CODE and MESSAGE match the given values. */ + +static void +assert_is_failure (const json::value *response, int code, const char *message) +{ + const json::object *obj = assert_is_response (response); + ASSERT_EQ (NULL, obj->get ("result")); + const json::object *err = obj->get ("error")->as_object (); + ASSERT_EQ (code, err->get ("code")->as_number ()->get ()); + ASSERT_STREQ (message, err->get ("message")->as_string ()->get_string ()); +} + using namespace jsonrpc; namespace { @@ -256,7 +292,7 @@ class test_server : public jsonrpc::server static void test_simple () { - test_server s (false); + test_server s (true); const char *request = ("{\"jsonrpc\": \"2.0\", \"method\": \"subtract\"," " \"params\": [42, 23], \"id\": 1}"); @@ -315,6 +351,17 @@ test_method_not_found () delete response; } +static void +test_malformed_json () +{ + test_server s (false); + const char *request = "{"; + json::value *response = s.handle_request_string (request); + assert_is_failure (response, PARSE_ERROR, + "error at index 1: expected string for object key"); + delete response; +} + /* Run all of the selftests within this file. */ void @@ -325,6 +372,7 @@ json_rpc_c_tests () test_bad_version (); test_method_not_a_string (); test_method_not_found (); + test_malformed_json (); // TODO: bogus JSON } diff --git a/gcc/json-rpc.h b/gcc/json-rpc.h index b1bdb10..4b5fd4b 100644 --- a/gcc/json-rpc.h +++ b/gcc/json-rpc.h @@ -52,7 +52,7 @@ const int METHOD_NOT_FOUND = -32601; const int INVALID_PARAMS = -32602; const int INTERNAL_ERROR = -32603; -extern json::value *make_parse_error (); +extern json::value *make_parse_error (const char *msg); extern json::value *make_invalid_request (const json::value *id); diff --git a/gcc/json.c b/gcc/json.c index 400a636..542b936 100644 --- a/gcc/json.c +++ b/gcc/json.c @@ -164,7 +164,7 @@ string::clone () const void literal::dump (FILE *outf) const { - fprintf (outf, "FIXME"); // FIXME: escaping + fprintf (outf, "FIXME"); // FIXME } value * @@ -202,6 +202,22 @@ enum token_id TOK_NUMBER }; +static const char *token_id_name[] = { + "error", + "EOF", + "'['", + "'{'", + "']'", + "'}'", + "':'", + "','", + "'true'", + "'false'", + "'null'", + "string", + "number" +}; + struct token { enum token_id id; @@ -219,7 +235,6 @@ class lexer public: lexer (); void add_utf8 (size_t sz, const char *utf8_buf); - void add_utf8 (const char *utf8); const token *peek (); void consume (); @@ -244,22 +259,21 @@ class lexer class parser { public: - parser (); + parser (char **err_out); void add_utf8 (size_t sz, const char *utf8_buf); - void add_utf8 (const char *utf8); value *parse_value (); object *parse_object (); array *parse_array (); - bool seen_error_p () const { return m_first_error; } + bool seen_error_p () const { return *m_err_out; } private: void require (enum token_id tok_id); - void error (const char *); + void error_at (int, const char *, ...); private: lexer m_lexer; - const char *m_first_error; + char **m_err_out; }; } // namespace json @@ -293,7 +307,7 @@ lexer::consume () gcc_assert (m_num_next_tokens > 0); gcc_assert (m_num_next_tokens <= MAX_TOKENS); - if (1) + if (0) { fprintf (stderr, "consuming token: "); dump_token (stderr, &m_next_tokens[0]); @@ -313,15 +327,7 @@ lexer::add_utf8 (size_t sz, const char *utf8_buf) { // FIXME: this assumes Latin-1. for (size_t i = 0; i < sz; i++) - m_buffer.safe_push (utf8_buf[sz]); -} - -void -lexer::add_utf8 (const char *utf8) -{ - // FIXME: this assumes Latin-1. - for (const char *ptr = utf8; *ptr; ptr++) - m_buffer.safe_push (*ptr); + m_buffer.safe_push (utf8_buf[i]); } bool @@ -406,12 +412,11 @@ lexer::dump_token (FILE *outf, const token *tok) void lexer::lex_token (token *out) { - out->index = m_next_char_idx; - /* Skip to next non-whitespace char. */ unichar next_char; while (1) { + out->index = m_next_char_idx; if (!get_char (next_char)) { out->id = TOK_EOF; @@ -566,9 +571,12 @@ lexer::lex_number (token *out, unichar first_char) out->u.number = value; } -parser::parser () -: m_lexer (), m_first_error (NULL) +parser::parser (char **err_out) +: m_lexer (), m_err_out (err_out) { + gcc_assert (err_out); + gcc_assert (*err_out == NULL); + *err_out = NULL; } void @@ -577,12 +585,6 @@ parser::add_utf8 (size_t sz, const char *utf8_buf) m_lexer.add_utf8 (sz, utf8_buf); } -void -parser::add_utf8 (const char *utf8) -{ - m_lexer.add_utf8 (utf8); -} - value * parser::parse_value () { @@ -604,8 +606,7 @@ parser::parse_value () return result; } - // FIXME - error ("error in parse_value"); + error_at (tok->index, "unexpected token: %s", token_id_name[tok->id]); return NULL; // FIXME } @@ -624,14 +625,14 @@ parser::parse_object () } if (tok->id != TOK_STRING) { - error ("expected string for object key"); + error_at (tok->index, "expected string for object key"); return result; } while (!seen_error_p ()) { tok = m_lexer.peek (); if (tok->id != TOK_STRING) - error ("expected string for object key"); + error_at (tok->index, "expected string for object key"); char *key = xstrdup (tok->u.string); m_lexer.consume (); @@ -696,32 +697,54 @@ parser::require (enum token_id tok_id) { const token *tok = m_lexer.peek (); if (tok->id != tok_id) - error ("expected FIXME"); + error_at (tok->index, "expected %s; got %s", token_id_name[tok_id], + token_id_name[tok->id]); m_lexer.consume (); } void -parser::error (const char *msg) +parser::error_at (int index, const char *fmt, ...) { - m_first_error = msg; - fprintf (stderr, "parsing error: %s\n", msg); + va_list ap; + va_start (ap, fmt); + char *formatted = xvasprintf (fmt, ap); + va_end (ap); + + char *msg_with_index = xasprintf ("error at index %i: %s", + index, formatted); + free (formatted); + + fprintf (stderr, "%s\n", msg_with_index); + if (*m_err_out == NULL) + *m_err_out = msg_with_index; + else + free (msg_with_index); } value * -json::parse_utf8_string (size_t sz, const char *utf8_buf) +json::parse_utf8_string (size_t sz, const char *utf8_buf, + char **err_out) { - parser p; + gcc_assert (err_out); + gcc_assert (*err_out == NULL); + + parser p (err_out); p.add_utf8 (sz, utf8_buf); - return p.parse_value (); + value *result = p.parse_value (); + if (p.seen_error_p ()) + { + gcc_assert (*err_out); + delete result; + return NULL; + } + return result; } value * -json::parse_utf8_string (const char *utf8) +json::parse_utf8_string (const char *utf8, char **err_out) { - parser p; - p.add_utf8 (utf8); - return p.parse_value (); + return parse_utf8_string (strlen (utf8), utf8, err_out); } @@ -734,7 +757,9 @@ namespace selftest { static void test_parse_string () { - json::value *jv = parse_utf8_string ("\"foo\""); + char *err = NULL; + json::value *jv = parse_utf8_string ("\"foo\"", &err); + ASSERT_EQ (NULL, err); ASSERT_EQ (JSON_STRING, jv->get_kind ()); ASSERT_STREQ ("foo", ((json::string *)jv)->get_string ()); delete jv; @@ -743,7 +768,9 @@ test_parse_string () static void test_parse_number () { - json::value *jv = parse_utf8_string ("42"); + char *err = NULL; + json::value *jv = parse_utf8_string ("42", &err); + ASSERT_EQ (NULL, err); ASSERT_EQ (JSON_NUMBER, jv->get_kind ()); ASSERT_EQ (42.0, ((json::number *)jv)->get ()); delete jv; @@ -752,7 +779,9 @@ test_parse_number () static void test_parse_array () { - json::value *jv = parse_utf8_string ("[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]"); + char *err = NULL; + json::value *jv = parse_utf8_string ("[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]", &err); + ASSERT_EQ (NULL, err); ASSERT_EQ (JSON_ARRAY, jv->get_kind ()); json::array *arr = static_cast (jv); ASSERT_EQ (10, arr->get_length ()); @@ -768,8 +797,10 @@ test_parse_array () static void test_parse_object () { + char *err = NULL; json::value *jv - = parse_utf8_string ("{\"foo\": \"bar\", \"baz\": [42, 43]}"); + = parse_utf8_string ("{\"foo\": \"bar\", \"baz\": [42, 43]}", &err); + ASSERT_EQ (NULL, err); ASSERT_TRUE (jv != NULL); ASSERT_EQ (JSON_OBJECT, jv->get_kind ()); json::object *jo = static_cast (jv); @@ -796,14 +827,53 @@ test_parse_object () } static void +test_parse_jsonrpc () +{ + char *err = NULL; + const char *request + = ("{\"jsonrpc\": \"2.0\", \"method\": \"subtract\"," + " \"params\": [42, 23], \"id\": 1}"); + json::value *jv = parse_utf8_string (request, &err); + ASSERT_EQ (NULL, err); + ASSERT_TRUE (jv != NULL); + delete jv; +} + +static void test_parse_empty_object () { - json::value *jv = parse_utf8_string ("{}"); + char *err = NULL; + json::value *jv = parse_utf8_string ("{}", &err); + ASSERT_EQ (NULL, err); ASSERT_TRUE (jv != NULL); ASSERT_EQ (JSON_OBJECT, jv->get_kind ()); delete jv; } +static void +test_error_empty_string () +{ + char *err = NULL; + json::value *jv = parse_utf8_string ("", &err); + ASSERT_STREQ ("error at index 0: unexpected token: EOF", err); + ASSERT_TRUE (jv == NULL); + free (err); +} + +static void +test_error_missing_comma () +{ + char *err = NULL; + /* 01234567. */ + const char *json = "[0, 1 2]"; + json::value *jv = parse_utf8_string (json, &err); + ASSERT_STREQ ("error at index 6: expected ']'; got number", + err); + // FIXME: unittest the lexer? + ASSERT_TRUE (jv == NULL); + free (err); +} + /* Run all of the selftests within this file. */ void @@ -813,7 +883,13 @@ json_c_tests () test_parse_number (); test_parse_array (); test_parse_object (); + test_parse_jsonrpc (); test_parse_empty_object (); + test_error_empty_string (); + test_error_missing_comma (); + + /* FIXME: tests for roundtripping (noting that we don't preserve + object key ordering). */ } } // namespace selftest diff --git a/gcc/json.h b/gcc/json.h index 0545068..f50e14f 100644 --- a/gcc/json.h +++ b/gcc/json.h @@ -148,8 +148,9 @@ class literal : public value /* Parsing decls. */ -extern value *parse_utf8_string (size_t sz, const char *utf8_buf); -extern value *parse_utf8_string (const char *utf8); +extern value *parse_utf8_string (size_t sz, const char *utf8_buf, + char **err_out); +extern value *parse_utf8_string (const char *utf8, char **err_out); } // namespace json diff --git a/gcc/lsp-main.c b/gcc/lsp-main.c index 4c2ab16..fd66683 100644 --- a/gcc/lsp-main.c +++ b/gcc/lsp-main.c @@ -28,7 +28,8 @@ along with GCC; see the file COPYING3. If not see class jsonrpc_server : public httpish_server { public: - jsonrpc_server (jsonrpc::server &handler) : m_handler (handler) {} + jsonrpc_server (jsonrpc::server &handler) + : httpish_server (true), m_handler (handler) {} void on_header (size_t key_sz, const char *key, size_t value_sz, const char *value) OVERRIDE FINAL @@ -99,5 +100,6 @@ serve_lsp (int port) { test_server ts (true); jsonrpc_server s (ts); + fprintf (stderr, "serving on port %i\n", port); s.serve (port); } diff --git a/gcc/testsuite/gcc.dg/lsp/test.py b/gcc/testsuite/gcc.dg/lsp/test.py new file mode 100644 index 0000000..9e36bf9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lsp/test.py @@ -0,0 +1,30 @@ +import jsonrpc # pip install json-rpc + +def test(): + print('test') + +import requests +import json + + +def main(): + url = "http://localhost:4000/jsonrpc" + headers = {'content-type': 'application/json'} + + # Example echo method + payload = { + "method": "echo", + "params": ["echome!"], + "jsonrpc": "2.0", + "id": 0, + } + response = requests.post( + url, data=json.dumps(payload), headers=headers).json() + + assert response["result"] == "echome!" + assert response["jsonrpc"] + assert response["id"] == 0 + +if __name__ == "__main__": + main() + -- 1.8.5.3