Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5c32c21

Browse files
committed
jsonapi: add lexer option to keep token ownership
Commit0785d1b adds support for libpq as a JSON client, butallocations for string tokens can still be leaked during parsingfailures. This is tricky to fix for the object_field semantic callbacks:the field name must remain valid until the end of the object, but if aparsing error is encountered partway through, object_field_end() won'tbe invoked and the client won't get a chance to free the field name.This patch adds a flag to switch the ownership of parsed tokens to thelexer. When this is enabled, the client must make a copy of any tokensit wants to persist past the callback lifetime, but the lexer willhandle necessary cleanup on failure.Backend uses of the JSON parser don't need to use this flag, since theparser's allocations will occur in a short lived memory context.A -o option has been added to test_json_parser_incremental to exercisethe new setJsonLexContextOwnsTokens() API, and the test_json_parser TAPtests make use of it. (The test program now cleans up allocated memory,so that tests can be usefully run under leak sanitizers.)Author: Jacob ChampionDiscussion:https://postgr.es/m/CAOYmi+kb38EciwyBQOf9peApKGwraHqA7pgzBkvoUnw5BRfS1g@mail.gmail.com
1 parent262283d commit5c32c21

File tree

6 files changed

+173
-33
lines changed

6 files changed

+173
-33
lines changed

‎src/common/jsonapi.c

Lines changed: 94 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ struct JsonParserStack
161161
*/
162162
structJsonIncrementalState
163163
{
164+
boolstarted;
164165
boolis_last_chunk;
165166
boolpartial_completed;
166167
jsonapi_StrValTypepartial_token;
@@ -280,6 +281,7 @@ static JsonParseErrorType parse_array_element(JsonLexContext *lex, const JsonSem
280281
staticJsonParseErrorTypeparse_array(JsonLexContext*lex,constJsonSemAction*sem);
281282
staticJsonParseErrorTypereport_parse_error(JsonParseContextctx,JsonLexContext*lex);
282283
staticboolallocate_incremental_state(JsonLexContext*lex);
284+
staticinlinevoidset_fname(JsonLexContext*lex,char*fname);
283285

284286
/* the null action object used for pure validation */
285287
constJsonSemActionnullSemAction=
@@ -437,7 +439,7 @@ allocate_incremental_state(JsonLexContext *lex)
437439
*fnull;
438440

439441
lex->inc_state=ALLOC0(sizeof(JsonIncrementalState));
440-
pstack=ALLOC(sizeof(JsonParserStack));
442+
pstack=ALLOC0(sizeof(JsonParserStack));
441443
prediction=ALLOC(JS_STACK_CHUNK_SIZE*JS_MAX_PROD_LEN);
442444
fnames=ALLOC(JS_STACK_CHUNK_SIZE*sizeof(char*));
443445
fnull=ALLOC(JS_STACK_CHUNK_SIZE*sizeof(bool));
@@ -464,10 +466,17 @@ allocate_incremental_state(JsonLexContext *lex)
464466
lex->pstack=pstack;
465467
lex->pstack->stack_size=JS_STACK_CHUNK_SIZE;
466468
lex->pstack->prediction=prediction;
467-
lex->pstack->pred_index=0;
468469
lex->pstack->fnames=fnames;
469470
lex->pstack->fnull=fnull;
470471

472+
/*
473+
* fnames between 0 and lex_level must always be defined so that
474+
* freeJsonLexContext() can handle them safely. inc/dec_lex_level() handle
475+
* the rest.
476+
*/
477+
Assert(lex->lex_level==0);
478+
lex->pstack->fnames[0]=NULL;
479+
471480
lex->incremental= true;
472481
return true;
473482
}
@@ -530,6 +539,25 @@ makeJsonLexContextIncremental(JsonLexContext *lex, int encoding,
530539
returnlex;
531540
}
532541

542+
void
543+
setJsonLexContextOwnsTokens(JsonLexContext*lex,boolowned_by_context)
544+
{
545+
if (lex->incremental&&lex->inc_state->started)
546+
{
547+
/*
548+
* Switching this flag after parsing has already started is a
549+
* programming error.
550+
*/
551+
Assert(false);
552+
return;
553+
}
554+
555+
if (owned_by_context)
556+
lex->flags |=JSONLEX_CTX_OWNS_TOKENS;
557+
else
558+
lex->flags &= ~JSONLEX_CTX_OWNS_TOKENS;
559+
}
560+
533561
staticinlinebool
534562
inc_lex_level(JsonLexContext*lex)
535563
{
@@ -569,12 +597,23 @@ inc_lex_level(JsonLexContext *lex)
569597
}
570598

571599
lex->lex_level+=1;
600+
601+
if (lex->incremental)
602+
{
603+
/*
604+
* Ensure freeJsonLexContext() remains safe even if no fname is
605+
* assigned at this level.
606+
*/
607+
lex->pstack->fnames[lex->lex_level]=NULL;
608+
}
609+
572610
return true;
573611
}
574612

575613
staticinlinevoid
576614
dec_lex_level(JsonLexContext*lex)
577615
{
616+
set_fname(lex,NULL);/* free the current level's fname, if needed */
578617
lex->lex_level-=1;
579618
}
580619

@@ -608,6 +647,15 @@ have_prediction(JsonParserStack *pstack)
608647
staticinlinevoid
609648
set_fname(JsonLexContext*lex,char*fname)
610649
{
650+
if (lex->flags&JSONLEX_CTX_OWNS_TOKENS)
651+
{
652+
/*
653+
* Don't leak prior fnames. If one hasn't been assigned yet,
654+
* inc_lex_level ensured that it's NULL (and therefore safe to free).
655+
*/
656+
FREE(lex->pstack->fnames[lex->lex_level]);
657+
}
658+
611659
lex->pstack->fnames[lex->lex_level]=fname;
612660
}
613661

@@ -655,8 +703,19 @@ freeJsonLexContext(JsonLexContext *lex)
655703
jsonapi_termStringInfo(&lex->inc_state->partial_token);
656704
FREE(lex->inc_state);
657705
FREE(lex->pstack->prediction);
706+
707+
if (lex->flags&JSONLEX_CTX_OWNS_TOKENS)
708+
{
709+
inti;
710+
711+
/* Clean up any tokens that were left behind. */
712+
for (i=0;i <=lex->lex_level;i++)
713+
FREE(lex->pstack->fnames[i]);
714+
}
715+
658716
FREE(lex->pstack->fnames);
659717
FREE(lex->pstack->fnull);
718+
FREE(lex->pstack->scalar_val);
660719
FREE(lex->pstack);
661720
}
662721

@@ -826,6 +885,7 @@ pg_parse_json_incremental(JsonLexContext *lex,
826885
lex->input=lex->token_terminator=lex->line_start=json;
827886
lex->input_length=len;
828887
lex->inc_state->is_last_chunk=is_last;
888+
lex->inc_state->started= true;
829889

830890
/* get the initial token */
831891
result=json_lex(lex);
@@ -1086,6 +1146,17 @@ pg_parse_json_incremental(JsonLexContext *lex,
10861146
if (sfunc!=NULL)
10871147
{
10881148
result= (*sfunc) (sem->semstate,pstack->scalar_val,pstack->scalar_tok);
1149+
1150+
/*
1151+
* Either ownership of the token passed to the
1152+
* callback, or we need to free it now. Either
1153+
* way, clear our pointer to it so it doesn't get
1154+
* freed in the future.
1155+
*/
1156+
if (lex->flags&JSONLEX_CTX_OWNS_TOKENS)
1157+
FREE(pstack->scalar_val);
1158+
pstack->scalar_val=NULL;
1159+
10891160
if (result!=JSON_SUCCESS)
10901161
returnresult;
10911162
}
@@ -1221,11 +1292,17 @@ parse_scalar(JsonLexContext *lex, const JsonSemAction *sem)
12211292
/* consume the token */
12221293
result=json_lex(lex);
12231294
if (result!=JSON_SUCCESS)
1295+
{
1296+
FREE(val);
12241297
returnresult;
1298+
}
12251299

1226-
/* invoke the callback */
1300+
/* invoke the callback, which may take ownership of val */
12271301
result= (*sfunc) (sem->semstate,val,tok);
12281302

1303+
if (lex->flags&JSONLEX_CTX_OWNS_TOKENS)
1304+
FREE(val);
1305+
12291306
returnresult;
12301307
}
12311308

@@ -1238,7 +1315,7 @@ parse_object_field(JsonLexContext *lex, const JsonSemAction *sem)
12381315
* generally call a field name a "key".
12391316
*/
12401317

1241-
char*fname=NULL;/* keep compiler quiet */
1318+
char*fname=NULL;
12421319
json_ofield_actionostart=sem->object_field_start;
12431320
json_ofield_actionoend=sem->object_field_end;
12441321
boolisnull;
@@ -1255,11 +1332,17 @@ parse_object_field(JsonLexContext *lex, const JsonSemAction *sem)
12551332
}
12561333
result=json_lex(lex);
12571334
if (result!=JSON_SUCCESS)
1335+
{
1336+
FREE(fname);
12581337
returnresult;
1338+
}
12591339

12601340
result=lex_expect(JSON_PARSE_OBJECT_LABEL,lex,JSON_TOKEN_COLON);
12611341
if (result!=JSON_SUCCESS)
1342+
{
1343+
FREE(fname);
12621344
returnresult;
1345+
}
12631346

12641347
tok=lex_peek(lex);
12651348
isnull=tok==JSON_TOKEN_NULL;
@@ -1268,7 +1351,7 @@ parse_object_field(JsonLexContext *lex, const JsonSemAction *sem)
12681351
{
12691352
result= (*ostart) (sem->semstate,fname,isnull);
12701353
if (result!=JSON_SUCCESS)
1271-
returnresult;
1354+
gotoofield_cleanup;
12721355
}
12731356

12741357
switch (tok)
@@ -1283,16 +1366,19 @@ parse_object_field(JsonLexContext *lex, const JsonSemAction *sem)
12831366
result=parse_scalar(lex,sem);
12841367
}
12851368
if (result!=JSON_SUCCESS)
1286-
returnresult;
1369+
gotoofield_cleanup;
12871370

12881371
if (oend!=NULL)
12891372
{
12901373
result= (*oend) (sem->semstate,fname,isnull);
12911374
if (result!=JSON_SUCCESS)
1292-
returnresult;
1375+
gotoofield_cleanup;
12931376
}
12941377

1295-
returnJSON_SUCCESS;
1378+
ofield_cleanup:
1379+
if (lex->flags&JSONLEX_CTX_OWNS_TOKENS)
1380+
FREE(fname);
1381+
returnresult;
12961382
}
12971383

12981384
staticJsonParseErrorType

‎src/include/common/jsonapi.h

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,11 @@ typedef struct JsonIncrementalState JsonIncrementalState;
9292
* conjunction with token_start.
9393
*
9494
* JSONLEX_FREE_STRUCT/STRVAL are used to drive freeJsonLexContext.
95+
* JSONLEX_CTX_OWNS_TOKENS is used by setJsonLexContextOwnsTokens.
9596
*/
9697
#defineJSONLEX_FREE_STRUCT(1 << 0)
9798
#defineJSONLEX_FREE_STRVAL(1 << 1)
99+
#defineJSONLEX_CTX_OWNS_TOKENS(1 << 2)
98100
typedefstructJsonLexContext
99101
{
100102
constchar*input;
@@ -130,9 +132,10 @@ typedef JsonParseErrorType (*json_scalar_action) (void *state, char *token, Json
130132
* to doing a pure parse with no side-effects, and is therefore exactly
131133
* what the json input routines do.
132134
*
133-
* The 'fname' and 'token' strings passed to these actions are palloc'd.
134-
* They are not free'd or used further by the parser, so the action function
135-
* is free to do what it wishes with them.
135+
* By default, the 'fname' and 'token' strings passed to these actions are
136+
* palloc'd. They are not free'd or used further by the parser, so the action
137+
* function is free to do what it wishes with them. This behavior may be
138+
* modified by setJsonLexContextOwnsTokens().
136139
*
137140
* All action functions return JsonParseErrorType. If the result isn't
138141
* JSON_SUCCESS, the parse is abandoned and that error code is returned.
@@ -216,6 +219,25 @@ extern JsonLexContext *makeJsonLexContextIncremental(JsonLexContext *lex,
216219
intencoding,
217220
boolneed_escapes);
218221

222+
/*
223+
* Sets whether tokens passed to semantic action callbacks are owned by the
224+
* context (in which case, the callback must duplicate the tokens for long-term
225+
* storage) or by the callback (in which case, the callback must explicitly
226+
* free tokens to avoid leaks).
227+
*
228+
* By default, this setting is false: the callback owns the tokens that are
229+
* passed to it (and if parsing fails between the two object-field callbacks,
230+
* the field name token will likely leak). If set to true, tokens will be freed
231+
* by the lexer after the callback completes.
232+
*
233+
* Setting this to true is important for long-lived clients (such as libpq)
234+
* that must not leak memory during a parse failure. For a server backend using
235+
* memory contexts, or a client application which will exit on parse failure,
236+
* this setting is less critical.
237+
*/
238+
externvoidsetJsonLexContextOwnsTokens(JsonLexContext*lex,
239+
boolowned_by_context);
240+
219241
externvoidfreeJsonLexContext(JsonLexContext*lex);
220242

221243
/* lex one token */

‎src/test/modules/test_json_parser/t/001_test_json_parser_incremental.pl

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,24 @@
1313

1414
my$test_file ="$FindBin::RealBin/../tiny.json";
1515

16-
my@exes =
17-
("test_json_parser_incremental","test_json_parser_incremental_shlib");
16+
my@exes = (
17+
["test_json_parser_incremental", ],
18+
["test_json_parser_incremental","-o", ],
19+
["test_json_parser_incremental_shlib", ],
20+
["test_json_parser_incremental_shlib","-o", ]);
1821

1922
foreachmy$exe (@exes)
2023
{
21-
note"testing executable$exe";
24+
note"testing executable@$exe";
2225

2326
# Test the usage error
24-
my ($stdout,$stderr) = run_command([$exe,"-c", 10 ]);
27+
my ($stdout,$stderr) = run_command([@$exe,"-c", 10 ]);
2528
like($stderr,qr/Usage:/,'error message if not enough arguments');
2629

2730
# Test that we get success for small chunk sizes from 64 down to 1.
2831
for (my$size = 64;$size > 0;$size--)
2932
{
30-
($stdout,$stderr) = run_command([$exe,"-c",$size,$test_file ]);
33+
($stdout,$stderr) = run_command([@$exe,"-c",$size,$test_file ]);
3134

3235
like($stdout,qr/SUCCESS/,"chunk size$size: test succeeds");
3336
is($stderr,"","chunk size$size: no error output");

‎src/test/modules/test_json_parser/t/002_inline.pl

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use File::Tempqw(tempfile);
1414

1515
my$dir = PostgreSQL::Test::Utils::tempdir;
16-
my$exe;
16+
my@exe;
1717

1818
subtest
1919
{
@@ -35,7 +35,7 @@ sub test
3535

3636
foreachmy$size (reverse(1 ..$chunk))
3737
{
38-
my ($stdout,$stderr) = run_command([$exe,"-c",$size,$fname ]);
38+
my ($stdout,$stderr) = run_command([@exe,"-c",$size,$fname ]);
3939

4040
if (defined($params{error}))
4141
{
@@ -53,13 +53,16 @@ sub test
5353
}
5454
}
5555

56-
my@exes =
57-
("test_json_parser_incremental","test_json_parser_incremental_shlib");
56+
my@exes = (
57+
["test_json_parser_incremental", ],
58+
["test_json_parser_incremental","-o", ],
59+
["test_json_parser_incremental_shlib", ],
60+
["test_json_parser_incremental_shlib","-o", ]);
5861

5962
foreach (@exes)
6063
{
61-
$exe =$_;
62-
note"testing executable$exe";
64+
@exe =@$_;
65+
note"testing executable@exe";
6366

6467
test("number","12345");
6568
test("string",'"hello"');

‎src/test/modules/test_json_parser/t/003_test_semantic.pl

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,17 @@
1616
my$test_file ="$FindBin::RealBin/../tiny.json";
1717
my$test_out ="$FindBin::RealBin/../tiny.out";
1818

19-
my@exes =
20-
("test_json_parser_incremental","test_json_parser_incremental_shlib");
19+
my@exes = (
20+
["test_json_parser_incremental", ],
21+
["test_json_parser_incremental","-o", ],
22+
["test_json_parser_incremental_shlib", ],
23+
["test_json_parser_incremental_shlib","-o", ]);
2124

2225
foreachmy$exe (@exes)
2326
{
24-
note"testing executable$exe";
27+
note"testing executable@$exe";
2528

26-
my ($stdout,$stderr) = run_command([$exe,"-s",$test_file ]);
29+
my ($stdout,$stderr) = run_command([@$exe,"-s",$test_file ]);
2730

2831
is($stderr,"","no error output");
2932

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp