8000 http: abstract http parsing out of httpclient · libgit2/libgit2@d396819 · GitHub
[go: up one dir, main page]
Skip to content

Commit d396819

Browse files
ethomsonethomson
committed
http: abstract http parsing out of httpclient
Avoid #ifdef's in httpclient.c, and move http parsing into its own file.
1 parent 3599de9 commit d396819

File tree

5 files changed

+305
-155
lines changed

5 files changed

+305
-155
lines changed

‎cmake/SelectHTTPParser.cmake

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,32 @@
11
# Optional external dependency: http-parser
2-
if(USE_HTTP_PARSER STREQUAL "builtin")
3-
message(STATUS "support for bundled (legacy) http-parser explicitly requested")
2+
if(USE_HTTP_PARSER STREQUAL "http-parser")
3+
find_package(HTTPParser)
44

5-
add_subdirectory("${PROJECT_SOURCE_DIR}/deps/http-parser" "${PROJECT_BINARY_DIR}/deps/http-parser")
6-
list(APPEND LIBGIT2_DEPENDENCY_INCLUDES "${PROJECT_SOURCE_DIR}/deps/http-parser")
7-
list(APPEND LIBGIT2_DEPENDENCY_OBJECTS "$<TARGET_OBJECTS:http-parser>")
8-
add_feature_info(http-parser ON "http-parser support (bundled)")
9-
else()
10-
# By default, try to use system LLHTTP. Fall back to
11-
# system http-parser, and even to bundled http-parser
12-
# as a last resort.
13-
find_package(LLHTTP)
5+
if(HTTP_PARSER_FOUND AND HTTP_PARSER_VERSION_MAJOR EQUAL 2)
6+
list(APPEND LIBGIT2_SYSTEM_INCLUDES ${HTTP_PARSER_INCLUDE_DIRS})
7+
list(APPEND LIBGIT2_SYSTEM_LIBS ${HTTP_PARSER_LIBRARIES})
8+
list(APPEND LIBGIT2_PC_LIBS "-lhttp_parser")
9+
set(GIT_HTTPPARSER_HTTPPARSER 1)
10+
add_feature_info(http-parser ON "using http-parser (system)")
11+
else()
12+
message(FATAL_ERROR "http-parser support was requested but not found")
13+
endif()
14+
elseif(USE_HTTP_PARSER STREQUAL "llhttp")
15+
find_package(LLHTTP)
1416

1517
if(LLHTTP_FOUND AND LLHTTP_VERSION_MAJOR EQUAL 9)
16-
add_compile_definitions(USE_LLHTTP)
1718
list(APPEND LIBGIT2_SYSTEM_INCLUDES ${LLHTTP_INCLUDE_DIRS})
1819
list(APPEND LIBGIT2_SYSTEM_LIBS ${LLHTTP_LIBRARIES})
1920
list(APPEND LIBGIT2_PC_LIBS "-lllhttp")
20-
add_feature_info(llhttp ON "llhttp support (system)")
21+
set(GIT_HTTPPARSER_LLHTTP 1)
22+
add_feature_info(http-parser ON "using llhttp (system)")
2123
else()
22-
message(STATUS "llhttp support was requested but not found; checking (legacy) http-parser support")
23-
find_package(HTTPParser)
24-
25-
if(HTTP_PARSER_FOUND AND HTTP_PARSER_VERSION_MAJOR EQUAL 2)
26-
list(APPEND LIBGIT2_SYSTEM_INCLUDES ${HTTP_PARSER_INCLUDE_DIRS})
27-
list(APPEND LIBGIT2_SYSTEM_LIBS ${HTTP_PARSER_LIBRARIES})
28-
list(APPEND LIBGIT2_PC_LIBS "-lhttp_parser")
29-
add_feature_info(http-parser ON "http-parser support (system)")
30-
else()
31-
message(STATUS "neither llhttp nor http-parser support was found; proceeding with bundled (legacy) http-parser")
32-
33-
add_subdirectory("${PROJECT_SOURCE_DIR}/deps/http-parser" "${PROJECT_BINARY_DIR}/deps/http-parser")
34-
list(APPEND LIBGIT2_DEPENDENCY_INCLUDES "${PROJECT_SOURCE_DIR}/deps/http-parser")
35-
list(APPEND LIBGIT2_DEPENDENCY_OBJECTS "$<TARGET_OBJECTS:http-parser>")
36-
add_feature_info(http-parser ON "http-parser support (bundled)")
37-
endif()
38-
endif()
24+
message(FATAL_ERROR "llhttp support was requested but not found")
25+
endif()
26+
else()
27+
add_subdirectory("${PROJECT_SOURCE_DIR}/deps/http-parser" "${PROJECT_BINARY_DIR}/deps/http-parser")
28+
list(APPEND LIBGIT2_DEPENDENCY_INCLUDES "${PROJECT_SOURCE_DIR}/deps/http-parser")
29+
list(APPEND LIBGIT2_DEPENDENCY_OBJECTS "$<TARGET_OBJECTS:http-parser>")
30+
set(GIT_HTTPPARSER_BUILTIN 1)
31+
add_feature_info(http-parser ON "using bundled parser")
3932
endif()

‎src/libgit2/transports/httpclient.c

Lines changed: 53 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -8,31 +8,6 @@
88
#include "common.h"
99
#include "git2.h"
1010

11-
#ifdef USE_LLHTTP
12-
#include <llhttp.h>
13-
typedef llhttp_settings_t http_settings_t;
14-
typedef llhttp_t http_parser_t;
15-
GIT_INLINE(http_settings_t *) http_client_parser_settings(void);
16-
#define git_http_parser_init(parser) llhttp_init(parser, HTTP_RESPONSE, http_client_parser_settings())
17-
#define git_http_parser_pause(parser) llhttp_pause(parser)
18-
#define git_http_parser_resume(parser) llhttp_resume(parser)
19-
#define git_http_parser_errno(parser) parser.error
20-
#define git_http_should_keep_alive(parser) llhttp_should_keep_alive(parser)
21-
#define git_http_errno_description(parser, errno) llhttp_get_error_reason(parser)
22-
#else
23-
#include <http_parser.h>
24-
/* Legacy http-parser. */
25-
typedef http_parser_settings http_settings_t;
26-
typedef struct http_parser http_parser_t;
27-
GIT_INLINE(http_settings_t *) http_client_parser_settings(void);
28-
#define git_http_parser_init(parser) http_parser_init(parser, HTTP_RESPONSE)
29-
#define git_http_parser_pause(parser) http_parser_pause(parser, 1)
30-
#define git_http_parser_resume(parser) http_parser_pause(parser, 0)
31-
#define git_http_parser_errno(parser) parser.http_errno
32-
#define git_http_should_keep_alive(parser) http_should_keep_alive(parser)
33-
#define git_http_errno_description(parser, errno) http_errno_description(errno)
34-
#endif /* USE_LLHTTP */
35-
3611
#include "vector.h"
3712
#include "trace.h"
3813
#include "httpclient.h"
@@ -46,6 +21,7 @@ GIT_INLINE(http_settings_t *) http_client_parser_settings(void);
4621
#include "streams/socket.h"
4722
#include "streams/tls.h"
4823
#include "auth.h"
24+
#include "httpparser.h"
4925

5026
static git_http_auth_scheme auth_schemes[] = {
5127
{ GIT_HTTP_AUTH_NEGOTIATE, "Negotiate", GIT_CREDENTIAL_DEFAULT, git_http_auth_negotiate },
@@ -133,7 +109,7 @@ struct git_http_client {
133109
git_http_server_t current_server;
134110
http_client_state state;
135111

136-
http_parser_t parser;
112+
git_http_parser parser;
137113

138114
git_http_server server;
139115
git_http_server proxy;
@@ -179,7 +155,7 @@ void git_http_response_dispose(git_http_response *response)
179155
memset(response, 0, sizeof(git_http_response));
180156
}
181157

182-
static int on_header_complete(http_parser_t *parser)
158+
static int on_header_complete(git_http_parser *parser)
183159
{
184160
http_parser_context *ctx = (http_parser_context *) parser->data;
185161
git_http_client *client = ctx->client;
@@ -244,7 +220,7 @@ static int on_header_complete(http_parser_t *parser)
244220
return 0;
245221
}
246222

247-
static int on_header_field(http_parser_t *parser, const char *str, size_t len)
223+
static int on_header_field(git_http_parser *parser, const char *str, size_t len)
248224
{
249225
http_parser_context *ctx = (http_parser_context *) parser->data;
250226

@@ -279,7 +255,7 @@ static int on_header_field(http_parser_t *parser, const char *str, size_t len)
279255
return 0;
280256
}
281257

282-
static int on_header_value(http_parser_t *parser, const char *str, size_t len)
258+
static int on_header_value(git_http_parser *parser, const char *str, size_t len)
283259
{
284260
http_parser_context *ctx = (http_parser_context *) parser->data;
285261

@@ -367,7 +343,7 @@ static int resend_needed(git_http_client *client, git_http_response *response)
367343
return 0;
368344
}
369345

370-
static int on_headers_complete(http_parser_t *parser)
346+
static int on_headers_complete(git_http_parser *parser)
371347
{
372348
http_parser_context *ctx = (http_parser_context *) parser->data;
373349

@@ -389,8 +365,8 @@ static int on_headers_complete(http_parser_t *parser)
389365
return ctx->parse_status = PARSE_STATUS_ERROR;
390366
}
391367

392-
ctx->response->status = parser->status_code;
393-
ctx->client->keepalive = git_http_should_keep_alive(parser);
368+
ctx->response->status = git_http_parser_status_code(parser);
369+
ctx->client->keepalive = git_http_parser_keep_alive(parser);
394370

395371
/* Prepare for authentication */
396372
collect_authinfo(&ctx->response->server_auth_schemetypes,
@@ -403,28 +379,15 @@ static int on_headers_complete(http_parser_t *parser)
403379
ctx->response->resend_credentials = resend_needed(ctx->client,
404380
ctx->response);
405381

406-
#ifndef USE_LLHTTP
407-
/* Stop parsing. llhttp documentation says about llhttp_pause():
408-
* "Do not call this from user callbacks! User callbacks must
409-
* return HPE_PAUSED if pausing is required", so that's what
410-
* we will do, and call git_http_parser_pause() only for
411-
* http-parser. */
412-
git_http_parser_pause(parser);
413-
#endif
414-
415382
if (ctx->response->content_type || ctx->response->chunked)
416383
ctx->client->state = READING_BODY;
417384
else
418385
ctx->client->state = DONE;
419386

420-
#ifdef USE_LLHTTP
421-
return HPE_PAUSED;
422-
#else
423-
return 0;
424-
#endif
387+
return git_http_parser_pause(parser);
425388
}
426389

427-
static int on_body(http_parser_t *parser, const char *buf, size_t len)
390+
static int on_body(git_http_parser *parser, const char *buf, size_t len)
428391
{
429392
http_parser_context *ctx = (http_parser_context *) parser->data;
430393
size_t max_len;
@@ -446,7 +409,7 @@ static int on_body(http_parser_t *parser, const char *buf, size_t len)
446409
return 0;
447410
}
448411

449-
static int on_message_complete(http_parser_t *parser)
412+
static int on_message_complete(git_http_parser *parser)
450413
{
451414
http_parser_context *ctx = (http_parser_context *) parser->data;
452415

@@ -911,9 +874,29 @@ GIT_INLINE(int) server_setup_from_url(
911874
return 0;
912875
}
913876

877+
static bool parser_settings_initialized;
878+
static git_http_parser_settings parser_settings;
879+
880+
GIT_INLINE(git_http_parser_settings *) http_client_parser_settings(void)
881+
{
882+
if (!parser_settings_initialized) {
883+
parser_settings.on_header_field = on_header_field;
884+
parser_settings.on_header_value = on_header_value;
885+
parser_settings.on_headers_complete = on_headers_complete;
886+
parser_settings.on_body = on_body;
887+
parser_settings.on_message_complete = on_message_complete;
888+
889+
parser_settings_initialized = true;
890+
}
891+
892+
return &parser_settings;
893+
}
894+
914895
static void reset_parser(git_http_client *client)
915896
{
916-
git_http_parser_init(&client->parser);
897+
git_http_parser_init(&client->parser,
898+
GIT_HTTP_PARSER_RESPONSE,
899+
http_client_parser_settings());
917900
}
918901

919902
static int setup_hosts(
@@ -1156,64 +1139,9 @@ GIT_INLINE(int) client_read(git_http_client *client)
11561139
return (int)read_len;
11571140
}
11581141

1159-
static bool parser_settings_initialized;
1160-
static http_settings_t parser_settings;
1161-
1162-
static size_t git_http_parser_execute(http_parser_t *parser, const char* data, size_t len)
1163-
{
1164-
#ifdef USE_LLHTTP
1165-
llhttp_errno_t error;
1166-
size_t parsed_len;
1167-
1168-
/*
1169-
* Unlike http_parser, which returns the number of parsed
1170-
* bytes in the _execute() call, llhttp returns an error
1171-
* code.
1172-
*/
1173-
1174-
if (data == NULL || len == 0) {
1175-
error = llhttp_finish(parser);
1176-
} else {
1177-
error = llhttp_execute(parser, data, len);
1178-
}
1179-
1180-
parsed_len = len;
1181-
/*
1182-
* Adjust number of parsed bytes in case of error.
1183-
*/
1184-
if (error != HPE_OK) {
1185-
parsed_len = llhttp_get_error_pos(parser) - data;
1186-
1187-
/* This isn't a real pause, just a way to stop parsing early. */
1188-
if (error == HPE_PAUSED_UPGRADE) {
1189-
llhttp_resume_after_upgrade(parser);
1190-
}
1191-
}
1192-
1193-
return parsed_len;
1194-
#else
1195-
return http_parser_execute(parser, http_client_parser_settings(), data, len);
1196-
#endif
1197-
}
1198-
1199-
GIT_INLINE(http_settings_t *) http_client_parser_settings(void)
1200-
{
1201-
if (!parser_settings_initialized) {
1202-
parser_settings.on_header_field = on_header_field;
1203-
parser_settings.on_header_value = on_header_value;
1204-
parser_settings.on_headers_complete = on_headers_complete;
1205-
parser_settings.on_body = on_body;
1206-
parser_settings.on_message_complete = on_message_complete;
1207-
1208-
parser_settings_initialized = true;
1209-
}
1210-
1211-
return &parser_settings;
1212-
}
1213-
12141142
GIT_INLINE(int) client_read_and_parse(git_http_client *client)
12151143
{
1216-
http_parser_t *parser = &client->parser;
1144+
git_http_parser *parser = &client->parser;
12171145
http_parser_context *ctx = (http_parser_context *) parser->data;
12181146
unsigned char http_errno;
12191147
int read_len;
@@ -1230,7 +1158,7 @@ GIT_INLINE(int) client_read_and_parse(git_http_client *client)
12301158
parsed_len = git_http_parser_execute(parser,
12311159
client->read_buf.ptr,
12321160
client->read_buf.size);
1233-
http_errno = git_http_parser_errno(client->parser);
1161+
http_errno = git_http_parser_errno(parser);
12341162

12351163
if (parsed_len > INT_MAX) {
12361164
git_error_set(GIT_ERROR_HTTP, "unexpectedly large parse");
@@ -1249,37 +1177,37 @@ GIT_INLINE(int) client_read_and_parse(git_http_client *client)
12491177
* (This can happen in response to an expect/continue request,
12501178
* where the server gives you a 100 and 200 simultaneously.)
12511179
*/
1252-
if (http_errno == HPE_PAUSED) {
1253-
#ifndef USE_LLHTTP
1254-
/*
1255-
* http-parser has a "feature" where it will not deliver the
1256-
* final byte when paused in a callback. Consume that byte.
1257-
* https://github.com/nodejs/http-parser/issues/97
1258-
*/
1259-
GIT_ASSERT(client->read_buf.size > parsed_len);
1180+
if (http_errno == GIT_HTTP_PARSER_PAUSED) {
1181+
size_t additional_size;
12601182

1261-
#endif
12621183
git_http_parser_resume(parser);
12631184

1264-
#ifndef USE_LLHTTP
1265-
parsed_len += git_http_parser_execute(parser,
1266-
client->read_buf.ptr + parsed_len,
1267-
1);
1268-
#endif
1185+
/*
1186+
* http-parser has a "feature" where it will not deliver
1187+
* the final byte when paused in a callback. Consume
1188+
* that byte.
1189+
*/
1190+
if ((additional_size = git_http_parser_remain_after_pause(parser)) > 0) {
1191+
GIT_ASSERT((client->read_buf.size - parsed_len) >= additional_size);
1192+
1193+
parsed_len += git_http_parser_execute(parser,
1194+
client->read_buf.ptr + parsed_len,
1195+
additional_size);
1196+
}
12691197
}
12701198

12711199
/* Most failures will be reported in http_errno */
1272-
else if (git_http_parser_errno(client->parser) != HPE_OK) {
1200+
else if (git_http_parser_errno(parser) != GIT_HTTP_PARSER_OK) {
12731201
git_error_set(GIT_ERROR_HTTP, "http parser error: %s",
1274-
git_http_errno_description(parser, http_errno));
1202+
git_http_parser_errmsg(parser, http_errno));
12751203
return -1;
12761204
}
12771205

12781206
/* Otherwise we should have consumed the entire buffer. */
12791207
else if (parsed_len != client->read_buf.size) {
12801208
git_error_set(GIT_ERROR_HTTP,
12811209
"http parser did not consume entire buffer: %s",
1282-
git_http_errno_description(parser, http_errno));
1210+
git_http_parser_errmsg(parser, http_errno));
12831211
return -1;
12841212
}
12851213

@@ -1318,7 +1246,7 @@ static void complete_response_body(git_http_client *client)
13181246

13191247
/* If there was an error, just close the connection. */
13201248
if (client_read_and_parse(client) < 0 ||
1321-
parser_context.error != HPE_OK ||
1249+
parser_context.error != GIT_HTTP_PARSER_OK ||
13221250
(parser_context.parse_status != PARSE_STATUS_OK &&
13231251
parser_context.parse_status != PARSE_STATUS_NO_OUTPUT)) {
13241252
git_error_clear();
@@ -1596,7 +1524,7 @@ int git_http_client_skip_body(git_http_client *client)
15961524
do {
15971525
error = client_read_and_parse(client);
15981526

1599-
if (parser_context.error != HPE_OK ||
1527+
if (parser_context.error != GIT_HTTP_PARSER_OK ||
16001528
(parser_context.parse_status != PARSE_STATUS_OK &&
16011529
parser_context.parse_status != PARSE_STATUS_NO_OUTPUT)) {
16021530
git_error_set(GIT_ERROR_HTTP,

0 commit comments

Comments
 (0)
0