Skip to content

Commit 85bca21

Browse files
kocsismatekocsismate
committed
Review round 1 fixes
1 parent 155e070 commit 85bca21

File tree

4 files changed

+95
-101
lines changed

4 files changed

+95
-101
lines changed

‎Zend/zend_string.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,8 +596,8 @@ EMPTY_SWITCH_DEFAULT_CASE()
596596
_(ZEND_STR_SCHEME, "scheme") \
597597
_(ZEND_STR_HOST, "host") \
598598
_(ZEND_STR_PORT, "port") \
599-
_(ZEND_STR_USERINFO, "userinfo") \
600599
_(ZEND_STR_USER, "user") \
600+
_(ZEND_STR_USERINFO, "userinfo") \
601601
_(ZEND_STR_USERNAME, "username") \
602602
_(ZEND_STR_PASS, "pass") \
603603
_(ZEND_STR_PASSWORD, "password") \

‎ext/uri/php_uri.c‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -488,9 +488,9 @@ PHP_METHOD(Uri_Rfc3986_Uri, resolve)
488488
{
489489
zend_string *uri_str;
490490

491-
ZEND_PARSE_PARAMETERS_START(1, 1) \
492-
Z_PARAM_PATH_STR(uri_str) \
493-
ZEND_PARSE_PARAMETERS_END(); \
491+
ZEND_PARSE_PARAMETERS_START(1, 1)
492+
Z_PARAM_PATH_STR(uri_str)
493+
ZEND_PARSE_PARAMETERS_END();
494494

495495
zend_object *this_object = Z_OBJ_P(ZEND_THIS);
496496
uri_internal_t *internal_uri = uri_internal_from_obj(this_object);
@@ -850,6 +850,8 @@ static PHP_MINIT_FUNCTION(uri)
850850
return FAILURE;
851851
}
852852

853+
uriparser_module_init();
854+
853855
if (uri_handler_register(&lexbor_uri_handler) == FAILURE) {
854856
return FAILURE;
855857
}
@@ -874,10 +876,6 @@ static PHP_MSHUTDOWN_FUNCTION(uri)
874876

875877
PHP_RINIT_FUNCTION(uri)
876878
{
877-
if (uriparser_request_init() == FAILURE) {
878-
return FAILURE;
879-
}
880-
881879
if (lexbor_request_init() == FAILURE) {
882880
return FAILURE;
883881
}

‎ext/uri/php_uriparser.c‎

Lines changed: 87 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@ void *uriparser_parse_uri(const zend_string *uri_str, const void *base_url, zval
2424
static void uriparser_free_uri(void *uri);
2525
static void throw_invalid_uri_exception(void);
2626

27-
HashTable uriparser_property_handlers;
28-
UriMemoryManager uriparser_memory_manager;
29-
3027
static void uriparser_copy_text_range(UriTextRangeA *text_range, UriTextRangeA *new_text_range, bool use_safe)
3128
{
3229
if (text_range->first == NULL || text_range->afterLast == NULL || (text_range->first > text_range->afterLast && !use_safe)) {
@@ -99,34 +96,43 @@ static UriUriA *uriparser_copy_uri(UriUriA *uriparser_uri) // TODO add to uripar
9996

10097
static zend_result uriparser_normalize_uri(UriUriA *uriparser_uri)
10198
{
102-
if (uriNormalizeSyntaxExMmA(uriparser_uri, (unsigned int)-1, &uriparser_memory_manager) != URI_SUCCESS) {
99+
if (uriNormalizeSyntaxExA(uriparser_uri, (unsigned int)-1) != URI_SUCCESS) {
103100
return FAILURE;
104101
}
105102

106103
return SUCCESS;
107104
}
108105

109-
#define URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode) do { \
110-
if (read_mode == URI_COMPONENT_READ_RAW) { \
111-
uriparser_uri = (UriUriA *) uriparser_uris->uri; \
112-
} else if (read_mode == URI_COMPONENT_READ_NORMALIZED_UNICODE || read_mode == URI_COMPONENT_READ_NORMALIZED_ASCII) { \
113-
if (uriparser_uris->normalized_uri == NULL) { \
114-
uriparser_uris->normalized_uri = uriparser_copy_uri(uriparser_uris->uri); \
115-
if (uriparser_normalize_uri(uriparser_uris->normalized_uri) == FAILURE) { \
116-
return FAILURE; \
117-
} \
118-
} \
119-
uriparser_uri = uriparser_uris->normalized_uri; \
120-
} else { \
121-
ZEND_UNREACHABLE(); \
122-
} \
123-
} while (0)
106+
static UriUriA *uriparser_read_uri(uriparser_uris_t *uriparser_uris, uri_component_read_mode_t read_mode)
107+
{
108+
switch (read_mode) {
109+
case URI_COMPONENT_READ_RAW:
110+
return uriparser_uris->uri;
111+
case URI_COMPONENT_READ_NORMALIZED_ASCII:
112+
ZEND_FALLTHROUGH;
113+
case URI_COMPONENT_READ_NORMALIZED_UNICODE:
114+
if (uriparser_uris->normalized_uri == NULL) {
115+
uriparser_uris->normalized_uri = uriparser_copy_uri(uriparser_uris->uri);
116+
if (uriparser_normalize_uri(uriparser_uris->normalized_uri) == FAILURE) {
117+
uriFreeUriMembersA(uriparser_uris->normalized_uri);
118+
efree(uriparser_uris->normalized_uri);
119+
uriparser_uris->normalized_uri = NULL;
120+
121+
return NULL;
122+
}
123+
}
124+
125+
return uriparser_uris->normalized_uri;
126+
EMPTY_SWITCH_DEFAULT_CASE()
127+
}
128+
}
124129

125130
static zend_result uriparser_read_scheme(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
126131
{
127-
uriparser_uris_t *uriparser_uris = (uriparser_uris_t *) internal_uri->uri;
128-
UriUriA *uriparser_uri;
129-
URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode);
132+
UriUriA *uriparser_uri = uriparser_read_uri(internal_uri->uri, read_mode);
133+
if (UNEXPECTED(uriparser_uri == NULL)) {
134+
return FAILURE;
135+
}
130136

131137
if (uriparser_uri->scheme.first != NULL && uriparser_uri->scheme.afterLast != NULL) {
132138
zend_string *str = zend_string_init(uriparser_uri->scheme.first, uriparser_uri->scheme.afterLast - uriparser_uri->scheme.first, false);
@@ -140,9 +146,10 @@ static zend_result uriparser_read_scheme(const uri_internal_t *internal_uri, uri
140146

141147
zend_result uriparser_read_userinfo(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
142148
{
143-
uriparser_uris_t *uriparser_uris = (uriparser_uris_t *) internal_uri->uri;
144-
UriUriA *uriparser_uri;
145-
URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode);
149+
UriUriA *uriparser_uri = uriparser_read_uri(internal_uri->uri, read_mode);
150+
if (UNEXPECTED(uriparser_uri == NULL)) {
151+
return FAILURE;
152+
}
146153

147154
if (uriparser_uri->userInfo.first != NULL && uriparser_uri->userInfo.afterLast != NULL) {
148155
ZVAL_STRINGL(retval, uriparser_uri->userInfo.first, uriparser_uri->userInfo.afterLast - uriparser_uri->userInfo.first);
@@ -155,14 +162,17 @@ zend_result uriparser_read_userinfo(const uri_internal_t *internal_uri, uri_comp
155162

156163
static zend_result uriparser_read_username(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
157164
{
158-
uriparser_uris_t *uriparser_uris = (uriparser_uris_t *) internal_uri->uri;
159-
UriUriA *uriparser_uri;
160-
URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode);
165+
UriUriA *uriparser_uri = uriparser_read_uri(internal_uri->uri, read_mode);
166+
if (UNEXPECTED(uriparser_uri == NULL)) {
167+
return FAILURE;
168+
}
161169

162170
if (uriparser_uri->userInfo.first != NULL && uriparser_uri->userInfo.afterLast != NULL) {
163-
char *c = strchr(uriparser_uri->userInfo.first, ':');
164-
if (c == NULL && uriparser_uri->userInfo.afterLast - uriparser_uri->userInfo.first > 0) {
165-
ZVAL_STRINGL(retval, uriparser_uri->userInfo.first, uriparser_uri->userInfo.afterLast - uriparser_uri->userInfo.first);
171+
size_t length = uriparser_uri->userInfo.afterLast - uriparser_uri->userInfo.first;
172+
char *c = memchr(uriparser_uri->userInfo.first, ':', length);
173+
174+
if (c == NULL && length > 0) {
175+
ZVAL_STRINGL(retval, uriparser_uri->userInfo.first, length);
166176
} else if (c != NULL && c - uriparser_uri->userInfo.first > 0) {
167177
ZVAL_STRINGL(retval, uriparser_uri->userInfo.first, c - uriparser_uri->userInfo.first);
168178
} else {
@@ -177,12 +187,14 @@ static zend_result uriparser_read_username(const uri_internal_t *internal_uri, u
177187

178188
static zend_result uriparser_read_password(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
179189
{
180-
uriparser_uris_t *uriparser_uris = (uriparser_uris_t *) internal_uri->uri;
181-
UriUriA *uriparser_uri;
182-
URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode);
190+
UriUriA *uriparser_uri = uriparser_read_uri(internal_uri->uri, read_mode);
191+
if (UNEXPECTED(uriparser_uri == NULL)) {
192+
return FAILURE;
193+
}
183194

184195
if (uriparser_uri->userInfo.first != NULL && uriparser_uri->userInfo.afterLast != NULL) {
185-
char *c = strchr(uriparser_uri->userInfo.first, ':');
196+
const char *c = memchr(uriparser_uri->userInfo.first, ':', uriparser_uri->userInfo.afterLast - uriparser_uri->userInfo.first);
197+
186198
if (c != NULL && uriparser_uri->userInfo.afterLast - c - 1 > 0) {
187199
ZVAL_STRINGL(retval, c + 1, uriparser_uri->userInfo.afterLast - c - 1);
188200
} else {
@@ -197,9 +209,10 @@ static zend_result uriparser_read_password(const uri_internal_t *internal_uri, u
197209

198210
static zend_result uriparser_read_host(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
199211
{
200-
uriparser_uris_t *uriparser_uris = (uriparser_uris_t *) internal_uri->uri;
201-
UriUriA *uriparser_uri;
202-
URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode);
212+
UriUriA *uriparser_uri = uriparser_read_uri(internal_uri->uri, read_mode);
213+
if (UNEXPECTED(uriparser_uri == NULL)) {
214+
return FAILURE;
215+
}
203216

204217
if (uriparser_uri->hostText.first != NULL && uriparser_uri->hostText.afterLast != NULL && uriparser_uri->hostText.afterLast - uriparser_uri->hostText.first > 0) {
205218
if (uriparser_uri->hostData.ip6 != NULL) {
@@ -209,7 +222,7 @@ static zend_result uriparser_read_host(const uri_internal_t *internal_uri, uri_c
209222
smart_str_appendl(&host_str, uriparser_uri->hostText.first, uriparser_uri->hostText.afterLast - uriparser_uri->hostText.first);
210223
smart_str_appendc(&host_str, ']');
211224

212-
ZVAL_STR(retval, smart_str_extract(&host_str));
225+
ZVAL_NEW_STR(retval, smart_str_extract(&host_str));
213226
} else {
214227
ZVAL_STRINGL(retval, uriparser_uri->hostText.first, uriparser_uri->hostText.afterLast - uriparser_uri->hostText.first);
215228
}
@@ -236,26 +249,27 @@ static zend_result uriparser_read_port(const uri_internal_t *internal_uri, uri_c
236249

237250
static zend_result uriparser_read_path(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
238251
{
239-
uriparser_uris_t *uriparser_uris = (uriparser_uris_t *) internal_uri->uri;
240-
UriUriA *uriparser_uri;
241-
URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode);
252+
UriUriA *uriparser_uri = uriparser_read_uri(internal_uri->uri, read_mode);
253+
if (UNEXPECTED(uriparser_uri == NULL)) {
254+
return FAILURE;
255+
}
242256

243257
if (uriparser_uri->pathHead != NULL) {
244258
const UriPathSegmentA *p;
245259
smart_str str = {0};
246260

247261
if (uriparser_uri->absolutePath || uriIsHostSetA(uriparser_uri)) {
248-
smart_str_appends(&str, "/");
262+
smart_str_appendc(&str, '/');
249263
}
250264

251-
smart_str_appendl(&str, uriparser_uri->pathHead->text.first, (int) ((uriparser_uri->pathHead->text).afterLast - (uriparser_uri->pathHead->text).first));
265+
smart_str_appendl(&str, uriparser_uri->pathHead->text.first, (size_t) ((uriparser_uri->pathHead->text).afterLast - (uriparser_uri->pathHead->text).first));
252266

253267
for (p = uriparser_uri->pathHead->next; p != NULL; p = p->next) {
254-
smart_str_appends(&str, "/");
268+
smart_str_appendc(&str, '/');
255269
smart_str_appendl(&str, p->text.first, (int) ((p->text).afterLast - (p->text).first));
256270
}
257271

258-
ZVAL_STR(retval, smart_str_extract(&str));
272+
ZVAL_NEW_STR(retval, smart_str_extract(&str));
259273
} else if (uriparser_uri->absolutePath) {
260274
ZVAL_CHAR(retval, '/');
261275
} else {
@@ -267,9 +281,10 @@ static zend_result uriparser_read_path(const uri_internal_t *internal_uri, uri_c
267281

268282
static zend_result uriparser_read_query(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
269283
{
270-
uriparser_uris_t *uriparser_uris = (uriparser_uris_t *) internal_uri->uri;
271-
UriUriA *uriparser_uri;
272-
URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode);
284+
UriUriA *uriparser_uri = uriparser_read_uri(internal_uri->uri, read_mode);
285+
if (UNEXPECTED(uriparser_uri == NULL)) {
286+
return FAILURE;
287+
}
273288

274289
if (uriparser_uri->query.first != NULL && uriparser_uri->query.afterLast != NULL) {
275290
ZVAL_STRINGL(retval, uriparser_uri->query.first, uriparser_uri->query.afterLast - uriparser_uri->query.first);
@@ -282,9 +297,10 @@ static zend_result uriparser_read_query(const uri_internal_t *internal_uri, uri_
282297

283298
static zend_result uriparser_read_fragment(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
284299
{
285-
uriparser_uris_t *uriparser_uris = (uriparser_uris_t *) internal_uri->uri;
286-
UriUriA *uriparser_uri;
287-
URIPARSER_READ_URI(uriparser_uri, uriparser_uris, read_mode);
300+
UriUriA *uriparser_uri = uriparser_read_uri(internal_uri->uri, read_mode);
301+
if (UNEXPECTED(uriparser_uri == NULL)) {
302+
return FAILURE;
303+
}
288304

289305
if (uriparser_uri->fragment.first != NULL && uriparser_uri->fragment.afterLast != NULL) {
290306
ZVAL_STRINGL(retval, uriparser_uri->fragment.first, uriparser_uri->fragment.afterLast - uriparser_uri->fragment.first);
@@ -307,38 +323,26 @@ static void *uriparser_calloc(UriMemoryManager *memory_manager, size_t nmemb, si
307323

308324
static void *uriparser_realloc(UriMemoryManager *memory_manager, void *ptr, size_t size)
309325
{
310-
return realloc(ptr, size);
326+
return erealloc(ptr, size);
311327
}
312328

313329
static void *uriparser_reallocarray(UriMemoryManager *memory_manager, void *ptr, size_t nmemb, size_t size)
314330
{
315-
const size_t total_size = nmemb * size;
316-
317-
/* check for unsigned overflow */
318-
if ((nmemb != 0) && (total_size / nmemb != size)) {
319-
errno = ENOMEM;
320-
return NULL;
321-
}
322-
323-
return erealloc(ptr, total_size);
331+
return safe_erealloc(ptr, nmemb, size, 0);
324332
}
325333

326334
static void uriparser_free(UriMemoryManager *memory_manager, void *ptr)
327335
{
328336
efree(ptr);
329337
}
330338

331-
zend_result uriparser_request_init(void)
339+
void uriparser_module_init(void)
332340
{
333-
uriparser_memory_manager.calloc = uriparser_calloc;
334-
uriparser_memory_manager.malloc = uriparser_malloc;
335-
uriparser_memory_manager.realloc = uriparser_realloc;
336-
uriparser_memory_manager.reallocarray = uriparser_reallocarray;
337-
uriparser_memory_manager.free = uriparser_free;
338-
339-
zend_hash_init(&uriparser_property_handlers, 8, NULL, NULL, true);
340-
341-
return SUCCESS;
341+
defaultMemoryManager.malloc = uriparser_malloc;
342+
defaultMemoryManager.calloc = uriparser_calloc;
343+
defaultMemoryManager.realloc = uriparser_realloc;
344+
defaultMemoryManager.reallocarray = uriparser_reallocarray;
345+
defaultMemoryManager.free = uriparser_free;
342346
}
343347

344348
static uriparser_uris_t *uriparser_create_uris(
@@ -357,16 +361,7 @@ static uriparser_uris_t *uriparser_create_uris(
357361

358362
static void throw_invalid_uri_exception(void)
359363
{
360-
zval exception;
361-
362-
object_init_ex(&exception, uri_invalid_uri_exception_ce);
363-
364-
zval value;
365-
ZVAL_STRING(&value, "URI parsing failed");
366-
zend_update_property_ex(uri_whatwg_invalid_url_exception_ce, Z_OBJ(exception), ZSTR_KNOWN(ZEND_STR_MESSAGE), &value);
367-
zval_ptr_dtor_str(&value);
368-
369-
zend_throw_exception_object(&exception);
364+
zend_throw_exception(uri_invalid_uri_exception_ce, "URI parsing failed", 0);
370365
}
371366

372367
void *uriparser_parse_uri_ex(const zend_string *uri_str, const uriparser_uris_t *uriparser_base_urls, bool silent)
@@ -376,7 +371,7 @@ void *uriparser_parse_uri_ex(const zend_string *uri_str, const uriparser_uris_t
376371
/* uriparser keeps the originally passed in string, while lexbor may allocate a new one. */
377372
zend_string *original_uri_str = zend_string_init(ZSTR_VAL(uri_str), ZSTR_LEN(uri_str), false);
378373
if (ZSTR_LEN(original_uri_str) == 0 ||
379-
uriParseSingleUriExMmA(uriparser_uri, ZSTR_VAL(original_uri_str), ZSTR_VAL(original_uri_str) + ZSTR_LEN(original_uri_str), NULL, &uriparser_memory_manager) != URI_SUCCESS
374+
uriParseSingleUriExA(uriparser_uri, ZSTR_VAL(original_uri_str), ZSTR_VAL(original_uri_str) + ZSTR_LEN(original_uri_str), NULL) != URI_SUCCESS
380375
) {
381376
efree(uriparser_uri);
382377
zend_string_release_ex(original_uri_str, false);
@@ -395,11 +390,11 @@ void *uriparser_parse_uri_ex(const zend_string *uri_str, const uriparser_uris_t
395390

396391
UriUriA *absolute_uri = emalloc(sizeof(UriUriA));
397392

398-
if (uriAddBaseUriExMmA(absolute_uri, uriparser_uri, uriparser_base_url, URI_RESOLVE_STRICTLY, &uriparser_memory_manager) != URI_SUCCESS) {
393+
if (uriAddBaseUriExA(absolute_uri, uriparser_uri, uriparser_base_url, URI_RESOLVE_STRICTLY) != URI_SUCCESS) {
399394
zend_string_release_ex(original_uri_str, false);
400-
uriFreeUriMembersMmA(uriparser_uri, &uriparser_memory_manager);
395+
uriFreeUriMembersA(uriparser_uri);
401396
efree(uriparser_uri);
402-
uriFreeUriMembersMmA(uriparser_base_url, &uriparser_memory_manager);
397+
uriFreeUriMembersA(uriparser_base_url);
403398
efree(uriparser_base_url);
404399
efree(absolute_uri);
405400

@@ -412,9 +407,9 @@ void *uriparser_parse_uri_ex(const zend_string *uri_str, const uriparser_uris_t
412407

413408
/* TODO fix freeing: if the following code runs, then we'll have use-after-free-s because uriparser doesn't
414409
copy the input. If we don't run the following code, then we'll have memory leaks...
415-
uriFreeUriMembersMmA(uriparser_base_url, &uriparser_memory_manager);
410+
uriFreeUriMembersA(uriparser_base_url);
416411
efree(uriparser_base_url);
417-
uriFreeUriMembersMmA(uriparser_uri, &uriparser_memory_manager);
412+
uriFreeUriMembersA(uriparser_uri);
418413
efree(uriparser_uri);
419414
*/
420415

@@ -462,12 +457,12 @@ static zend_string *uriparser_uri_to_string(void *uri, uri_recomposition_mode_t
462457

463458
zend_string *uri_string = zend_string_alloc(charsRequired - 1, false);
464459
if (uriToStringA(ZSTR_VAL(uri_string), uriparser_uri, charsRequired, NULL) != URI_SUCCESS) {
465-
zend_string_release(uri_string);
460+
zend_string_efree(uri_string);
466461
return NULL;
467462
}
468463

469464
if (exclude_fragment) {
470-
char *pos = strrchr(ZSTR_VAL(uri_string), '#');
465+
const char *pos = zend_memrchr(ZSTR_VAL(uri_string), '#', ZSTR_LEN(uri_string));
471466
if (pos != NULL) {
472467
uri_string = zend_string_truncate(uri_string, (pos - ZSTR_VAL(uri_string)), false);
473468
}
@@ -486,7 +481,7 @@ static void uriparser_free_uri(void *uri)
486481
uriparser_uris->uri_str = NULL;
487482
}
488483

489-
uriFreeUriMembersMmA(uriparser_uris->uri, &uriparser_memory_manager);
484+
uriFreeUriMembersA(uriparser_uris->uri);
490485
efree(uriparser_uris->uri);
491486
uriparser_uris->uri = NULL;
492487
}
@@ -498,7 +493,7 @@ static void uriparser_free_uri(void *uri)
498493
uriparser_uris->normalized_uri_str = NULL;
499494
}
500495

501-
uriFreeUriMembersMmA(uriparser_uris->normalized_uri, &uriparser_memory_manager);
496+
uriFreeUriMembersA(uriparser_uris->normalized_uri);
502497
efree(uriparser_uris->normalized_uri);
503498
uriparser_uris->normalized_uri = NULL;
504499
}

0 commit comments

Comments
 (0)