Skip to content

Commit 1340b32

Browse files
indutnyindutny
committed
api: allow returning -1 in spans, doc HPE_USER
`HPE_USER` return value was allowed in span callbacks (`on_body`, etc), but wasn't in the documentation. Additionally, looking at the return values of other documented callbacks it was easy to assume that `-1` was a valid return value for spans as well. Instead of returning it as it is change it to `HPE_USER` and add automatic error reason with the span's name. PR-URL: #104 Reviewed-By: Daniele Belardi <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
1 parent 3ec2251 commit 1340b32

2 files changed

Lines changed: 35 additions & 17 deletions

File tree

‎src/native/api.c‎

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,30 @@
44

55
#include "llhttp.h"
66

7-
#define CALLBACK_MAYBE(PARSER, NAME, ...) \
7+
#define CALLBACK_MAYBE(PARSER, NAME) \
88
do { \
99
const llhttp_settings_t* settings; \
1010
settings = (const llhttp_settings_t*) (PARSER)->settings; \
1111
if (settings == NULL || settings->NAME == NULL) { \
1212
err = 0; \
1313
break; \
1414
} \
15-
err = settings->NAME(__VA_ARGS__); \
15+
err = settings->NAME((PARSER)); \
16+
} while (0)
17+
18+
#define SPAN_CALLBACK_MAYBE(PARSER, NAME, START, LEN) \
19+
do { \
20+
const llhttp_settings_t* settings; \
21+
settings = (const llhttp_settings_t*) (PARSER)->settings; \
22+
if (settings == NULL || settings->NAME == NULL) { \
23+
err = 0; \
24+
break; \
25+
} \
26+
err = settings->NAME((PARSER), (START), (LEN)); \
27+
if (err == -1) { \
28+
err = HPE_USER; \
29+
llhttp_set_error_reason((PARSER), "Span callback error in " #NAME); \
30+
} \
1631
} while (0)
1732

1833
void llhttp_init(llhttp_t* parser, llhttp_type_t type,
@@ -123,7 +138,7 @@ llhttp_errno_t llhttp_finish(llhttp_t* parser) {
123138

124139
switch (parser->finish) {
125140
case HTTP_FINISH_SAFE_WITH_CB:
126-
CALLBACK_MAYBE(parser, on_message_complete, parser);
141+
CALLBACK_MAYBE(parser, on_message_complete);
127142
if (err != HPE_OK) return err;
128143

129144
/* FALLTHROUGH */
@@ -237,98 +252,98 @@ void llhttp_set_lenient_keep_alive(llhttp_t* parser, int enabled) {
237252

238253
int llhttp__on_message_begin(llhttp_t* s, const char* p, const char* endp) {
239254
int err;
240-
CALLBACK_MAYBE(s, on_message_begin, s);
255+
CALLBACK_MAYBE(s, on_message_begin);
241256
return err;
242257
}
243258

244259

245260
int llhttp__on_url(llhttp_t* s, const char* p, const char* endp) {
246261
int err;
247-
CALLBACK_MAYBE(s, on_url, s, p, endp - p);
262+
SPAN_CALLBACK_MAYBE(s, on_url, p, endp - p);
248263
return err;
249264
}
250265

251266

252267
int llhttp__on_url_complete(llhttp_t* s, const char* p, const char* endp) {
253268
int err;
254-
CALLBACK_MAYBE(s, on_url_complete, s);
269+
CALLBACK_MAYBE(s, on_url_complete);
255270
return err;
256271
}
257272

258273

259274
int llhttp__on_status(llhttp_t* s, const char* p, const char* endp) {
260275
int err;
261-
CALLBACK_MAYBE(s, on_status, s, p, endp - p);
276+
SPAN_CALLBACK_MAYBE(s, on_status, p, endp - p);
262277
return err;
263278
}
264279

265280

266281
int llhttp__on_status_complete(llhttp_t* s, const char* p, const char* endp) {
267282
int err;
268-
CALLBACK_MAYBE(s, on_status_complete, s);
283+
CALLBACK_MAYBE(s, on_status_complete);
269284
return err;
270285
}
271286

272287

273288
int llhttp__on_header_field(llhttp_t* s, const char* p, const char* endp) {
274289
int err;
275-
CALLBACK_MAYBE(s, on_header_field, s, p, endp - p);
290+
SPAN_CALLBACK_MAYBE(s, on_header_field, p, endp - p);
276291
return err;
277292
}
278293

279294

280295
int llhttp__on_header_field_complete(llhttp_t* s, const char* p, const char* endp) {
281296
int err;
282-
CALLBACK_MAYBE(s, on_header_field_complete, s);
297+
CALLBACK_MAYBE(s, on_header_field_complete);
283298
return err;
284299
}
285300

286301

287302
int llhttp__on_header_value(llhttp_t* s, const char* p, const char* endp) {
288303
int err;
289-
CALLBACK_MAYBE(s, on_header_value, s, p, endp - p);
304+
SPAN_CALLBACK_MAYBE(s, on_header_value, p, endp - p);
290305
return err;
291306
}
292307

293308

294309
int llhttp__on_header_value_complete(llhttp_t* s, const char* p, const char* endp) {
295310
int err;
296-
CALLBACK_MAYBE(s, on_header_value_complete, s);
311+
CALLBACK_MAYBE(s, on_header_value_complete);
297312
return err;
298313
}
299314

300315

301316
int llhttp__on_headers_complete(llhttp_t* s, const char* p, const char* endp) {
302317
int err;
303-
CALLBACK_MAYBE(s, on_headers_complete, s);
318+
CALLBACK_MAYBE(s, on_headers_complete);
304319
return err;
305320
}
306321

307322

308323
int llhttp__on_message_complete(llhttp_t* s, const char* p, const char* endp) {
309324
int err;
310-
CALLBACK_MAYBE(s, on_message_complete, s);
325+
CALLBACK_MAYBE(s, on_message_complete);
311326
return err;
312327
}
313328

314329

315330
int llhttp__on_body(llhttp_t* s, const char* p, const char* endp) {
316331
int err;
317-
CALLBACK_MAYBE(s, on_body, s, p, endp - p);
332+
SPAN_CALLBACK_MAYBE(s, on_body, p, endp - p);
318333
return err;
319334
}
320335

321336

322337
int llhttp__on_chunk_header(llhttp_t* s, const char* p, const char* endp) {
323338
int err;
324-
CALLBACK_MAYBE(s, on_chunk_header, s);
339+
CALLBACK_MAYBE(s, on_chunk_header);
325340
return err;
326341
}
327342

328343

329344
int llhttp__on_chunk_complete(llhttp_t* s, const char* p, const char* endp) {
330345
int err;
331-
CALLBACK_MAYBE(s, on_chunk_complete, s);
346+
CALLBACK_MAYBE(s, on_chunk_complete);
332347
return err;
333348
}
334349

‎src/native/api.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ struct llhttp_settings_s {
2121
/* Possible return values 0, -1, `HPE_PAUSED` */
2222
llhttp_cb on_message_begin;
2323

24+
/* Possible return values 0, -1, HPE_USER */
2425
llhttp_data_cb on_url;
2526
llhttp_data_cb on_status;
2627
llhttp_data_cb on_header_field;
@@ -37,6 +38,7 @@ struct llhttp_settings_s {
3738
*/
3839
llhttp_cb on_headers_complete;
3940

41+
/* Possible return values 0, -1, HPE_USER */
4042
llhttp_data_cb on_body;
4143

4244
/* Possible return values 0, -1, `HPE_PAUSED` */
@@ -49,6 +51,7 @@ struct llhttp_settings_s {
4951
llhttp_cb on_chunk_header;
5052
llhttp_cb on_chunk_complete;
5153

54+
/* Information-only callbacks, return value is ignored */
5255
llhttp_cb on_url_complete;
5356
llhttp_cb on_status_complete;
5457
llhttp_cb on_header_field_complete;

0 commit comments

Comments
 (0)