Use add_assoc_zval_ex#7
Merged
Merged
Conversation
Fixes two critical memory management bugs:
1. Double-free in Request::create() with Uri objects (src/request.c:909-914)
- Removed erroneous smart_str_free() after ZVAL_STR_COPY()
- ZVAL_STR_COPY already takes ownership via refcounting
- The smart_str_free() was prematurely freeing memory still referenced by intern->zv_uri
- Caused 'zend_mm_heap corrupted' crash at test 63/210 in CI on PHP 8.5.3-dev
2. Memory leak in Response::create() with string bodies (src/response.c:388)
- Added missing zval_ptr_dtor(&body_zv) to clean up copied parameter
- ZVAL_COPY increments refcount, must be balanced with zval_ptr_dtor
Security Fixes:
- Fix use-after-free in Stream::getContents() and __toString() by adding
zend_string_copy() to properly increment refcounts before returning strings
- Add integer overflow/underflow checks in Stream::seek() for SEEK_CUR and
SEEK_END operations to prevent wraparound vulnerabilities
- Add NULL check in Request::getHeaderLine() to prevent crashes when
smart_str_0() returns NULL for empty header arrays
- Replace unsafe atol() with ZEND_STRTOL() in UploadedFile for safe
string-to-long conversion with proper error handling and bounds checking
Build System Improvements:
- Add VERSION build argument to Dockerfile (83, 84, 85) for multi-version
support, defaulting to PHP 8.5
- Update Makefile to use VERSION variable consistently across all targets
(docker-build, docker-test, docker-shell, ci-test-all)
- Remove build-and-push.sh script and associated Makefile target
- Properly format VERSION to PHP branch (85 -> PHP-8.5) using shell parsing
Configuration:
- Enable -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 unconditionally in config.m4
for better performance on modern PHP 7.0+ (recommended best practice)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changed from:
zend_hash_add(Z_ARRVAL_P(return_value), key, &header_array);
To:
add_assoc_zval_ex(return_value, ZSTR_VAL(key), ZSTR_LEN(key), &header_array);
This creates a fresh key string from the raw char* and length, ensuring proper key ownership in the return value array.
Files Modified