fix: check parameters in json-write.c
PATCH 1/1 fix: check parameters in json-write.c
From: wangsirun <[email protected]>
When I used the json-writer.c file as a lib, I found that it often caused coredump errors, so I submitted this patch
---
fix: check parameters in json-write.c
When I used the json-writer.c file as a lib, I found that it often caused coredump errors, so I submitted this patch
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1576%2Fwangsirun%2Fjson-write-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1576/wangsirun/json-write-fix-v1
Pull-Request: https://github.com/git/git/pull/1576
json-writer.c | 5 +++++ 1 file changed, 5 insertions(+)
json-writer.c +5 -0
@@ -20,6 +20,11 @@ static void append_quoted_string(struct strbuf *out, const char *in) {unsigned char c; +if (!in || !*in) { +strbuf_addstr (out,"\"\"" ); +return ; + } +strbuf_addch (out,'"' );while ((c = *in++) !='\0' ) {if (c =='"' )
base-commit: d4a83d07b8cc66d4afac2f33a8af729f2ba93bba
Signature · 1 line
gitgitgadget
[+cc Jeff Hostetler]
json-writer.c static void append_quoted_string(struct strbuf *out, const char *in) · line 26
Quoted 12 lines · mark via GitGitGadget
json-writer.c +4
-0@@ -20,6 +20,11 @@ static void append_quoted_string(struct strbuf *out, const char *in) {unsigned char c; +if (!in || !*in) { +strbuf_addstr (out,"\"\"" ); +return ; + }
From reading the implementation of append_quoted_string(), I think that the case where "in" is the empty string is already covered. IOW, doing something like:
struct strbuf buf = STRBUF_INIT;
append_quoted_string(&out, "");
warning("'%s'", buf.buf);would print out something like:
warning: '""'
as expected. Handling a NULL "in" argument is new behavior, but I am not sure if it is appropriate to coerce a NULL input into the empty string. I've CC'd the author of this code, whose opinion I trust more than my own here.
Thanks,
Taylor
Quoted 28 lines · Taylor Blau
[+cc Jeff Hostetler]
json-writer.c static void append_quoted_string(struct strbuf *out, const char *in) · line 26
Quoted 12 lines · mark via GitGitGadget
json-writer.c +4
-0@@ -20,6 +20,11 @@ static void append_quoted_string(struct strbuf *out, const char *in) {unsigned char c; +if (!in || !*in) { +strbuf_addstr (out,"\"\"" ); +return ; + }From reading the implementation of
append_quoted_string(), I think that
the case where "in" is the empty string is already covered. IOW, doing something like:struct strbuf buf = STRBUF_INIT; append_quoted_string(&out, ""); warning("'%s'", buf.buf);would print out something like:
warning: '""'
Quoted 7 lines
as expected. Handling a NULL "in" argument is new behavior, but I am not sure if it is appropriate to coerce a NULL input into the empty string. I've CC'd the author of this code, whose opinion I trust more than my own here.
Thanks,
Taylor
There are three callers of append_quoted_string() and it is static to the json-writer.c code.
Basically, in a JSON object, we have 2 uses:
{
"<key>" : "<string-value>",
"<key>" : <integer>,
...
}And in a JSON array, we have the other:
[
"<string-value>",
...
]I suppose it is OK for the 2 string-value cases to assume a NULL pointer could be written as "" in the JSON output. Although, I kinda think a NULL pointer should call BUG() as we have in the various assert_*() routines. It really is a kind of logic error in the caller.
Regardless what we decide for the <string-value> case, in the <key> case, the resulting JSON would not be valid. We need for the key to be a non-empty string. For example { "" : 1 } is not valid JSON. So the key case should call BUG() and not try to hide it.
So I'm leaning towards just making it a BUG() in all cases, but I'm open to the other mixed handling.
Jeff
Quoted 4 lines · Jeff Hostetler
I suppose it is OK for the 2 string-value cases to assume a NULL pointer could be written as "" in the JSON output. Although, I kinda think a NULL pointer should call
BUG()as we have in the various assert_*() routines. It really is a kind of logic error in the caller.
FWIW, that is my preference, too.
Quoted 4 lines
Regardless what we decide for the <string-value> case, in the <key> case, the resulting JSON would not be valid. We need for the key to be a non-empty string. For example { "" : 1 } is not valid JSON. So the key case should call
BUG()and not try to hide it.
I do not have a strong opinion on this side, and leave it up to the area experts ;-)
Quoted 5 lines
So I'm leaning towards just making it a
BUG()in all cases, but I'm open to the other mixed handling.Jeff