Skip to content

Commit f3112a6

Browse files
yordadevyordadev
authored
Heap Corruption, Security & Build System Improvements (#6)
Build System: Standardized PHP version notation (8.3, 8.4, 8.5 instead of 83, 84, 85) Docker: Added explicit platform specification (--platform linux/amd64) for consistency Valgrind: Enhanced Dockerfile.valgrind to mirror CI Ubuntu 24.04 environment Fixes critical heap corruption bug in PHP 8.5.3-dev when creating Request objects with Uri object parameters. The issue caused zend_mm_heap corrupted errors and test failures in CI.
1 parent da8b8eb commit f3112a6

14 files changed

Lines changed: 1311 additions & 112 deletions

‎Dockerfile‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,17 @@
22
# Mirrors php-builds approach: compiles PHP from source then builds extension
33
#
44
# Build args:
5-
# VERSION - PHP version (83, 84, 85) - defaults to 85 (PHP 8.5)
5+
# PHP_VERSION - PHP version (8.3, 8.4, 8.5) - defaults to 8.5
66
#
7-
ARG VERSION=85
7+
ARG PHP_VERSION=8.5
88

99
FROM ubuntu:24.04
1010

11-
ARG VERSION
11+
ARG PHP_VERSION
1212
ENV DEBIAN_FRONTEND=noninteractive
13-
ENV PHP_VERSION=${VERSION}
1413

1514
LABEL maintainer="Signalforge Team"
16-
LABEL description="PHP with signalforge_http extension"
15+
LABEL description="PHP ${PHP_VERSION} with signalforge_http extension"
1716
LABEL php.version="${PHP_VERSION}"
1817

1918
# Install build dependencies
@@ -45,8 +44,8 @@ RUN apt-get update -qq && apt-get install -y -qq --no-install-recommends \
4544

4645
# Clone and build PHP from source
4746
WORKDIR /tmp
48-
RUN MAJOR=$(echo ${PHP_VERSION} | cut -c1); \
49-
MINOR=$(echo ${PHP_VERSION} | cut -c2); \
47+
RUN MAJOR=$(echo ${PHP_VERSION} | cut -d. -f1); \
48+
MINOR=$(echo ${PHP_VERSION} | cut -d. -f2); \
5049
PHP_BRANCH="PHP-${MAJOR}.${MINOR}"; \
5150
git clone --depth 1 --branch ${PHP_BRANCH} --quiet https://github.com/php/php-src.git
5251

@@ -71,8 +70,8 @@ RUN ./configure --quiet \
7170
--enable-bcmath \
7271
--with-readline
7372

74-
RUN make -j$(nproc) -s
75-
RUN make install -s
73+
RUN make -j$(nproc)
74+
RUN make install
7675

7776
# Create extension config directory
7877
RUN mkdir -p /usr/local/etc/php/conf.d

‎Dockerfile.valgrind‎

Lines changed: 80 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,99 @@
11
# Dockerfile.valgrind - For memory leak detection
22
# Build with debug symbols and no optimization for meaningful Valgrind output
3+
# Mirrors CI environment: Ubuntu 24.04 + PHP built from source
34

4-
ARG PHP_VERSION=8.4
5+
ARG PHP_VERSION=8.5
56

6-
FROM php:${PHP_VERSION}-cli-alpine
7+
FROM ubuntu:24.04
8+
9+
ARG PHP_VERSION
10+
ENV DEBIAN_FRONTEND=noninteractive
11+
12+
LABEL maintainer="Signalforge Team"
13+
LABEL description="PHP ${PHP_VERSION} with signalforge_http extension (Debug + Valgrind)"
14+
LABEL php.version="${PHP_VERSION}"
715

816
# Install build dependencies AND valgrind
9-
RUN apk add --no-cache \
10-
autoconf g++ gcc make pkgconf re2c linux-headers valgrind ${PHPIZE_DEPS}
17+
RUN apt-get update -qq && apt-get install -y -qq --no-install-recommends \
18+
git \
19+
gcc \
20+
g++ \
21+
make \
22+
autoconf \
23+
automake \
24+
libtool \
25+
pkg-config \
26+
re2c \
27+
bison \
28+
wget \
29+
valgrind \
30+
libxml2-dev \
31+
libssl-dev \
32+
libcurl4-openssl-dev \
33+
libzip-dev \
34+
libonig-dev \
35+
libsqlite3-dev \
36+
libpq-dev \
37+
libreadline-dev \
38+
libpcre2-dev \
39+
libsodium-dev \
40+
zlib1g-dev \
41+
ca-certificates \
42+
&& rm -rf /var/lib/apt/lists/*
1143

12-
WORKDIR /build
44+
# Clone and build PHP from source (with debug symbols)
45+
WORKDIR /tmp
46+
RUN MAJOR=$(echo ${PHP_VERSION} | cut -d. -f1); \
47+
MINOR=$(echo ${PHP_VERSION} | cut -d. -f2); \
48+
PHP_BRANCH="PHP-${MAJOR}.${MINOR}"; \
49+
git clone --depth 1 --branch ${PHP_BRANCH} --quiet https://github.com/php/php-src.git
50+
51+
WORKDIR /tmp/php-src
52+
RUN ./buildconf --force > /dev/null
53+
54+
# Configure PHP (same as Dockerfile, debug symbols will come from CFLAGS)
55+
RUN CFLAGS="-g" ./configure --quiet \
56+
--prefix=/usr/local \
57+
--with-config-file-path=/usr/local/etc/php \
58+
--with-config-file-scan-dir=/usr/local/etc/php/conf.d \
59+
--with-curl \
60+
--with-openssl \
61+
--with-zip \
62+
--with-zlib \
63+
--enable-mbstring \
64+
--enable-opcache \
65+
--with-pdo-mysql \
66+
--with-pdo-pgsql \
67+
--with-mysqli \
68+
--enable-sockets \
69+
--enable-pcntl \
70+
--enable-bcmath \
71+
--with-readline
72+
73+
RUN make -j$(nproc)
74+
RUN make install
75+
76+
# Create extension config directory
77+
RUN mkdir -p /usr/local/etc/php/conf.d
1378

14-
# Copy extension source
79+
# Copy and build http extension with debug symbols (no optimization for accurate valgrind)
80+
WORKDIR /build
1581
COPY . /build
1682

17-
# Build the extension with debug symbols (critical for Valgrind)
18-
RUN phpize \
83+
RUN /usr/local/bin/phpize \
1984
&& CFLAGS="-g -O0" ./configure --enable-signalforge_http \
20-
&& make \
21-
&& make install \
22-
&& docker-php-ext-enable signalforge_http
85+
&& make -j$(nproc) \
86+
&& make install
2387

24-
# Get run-tests.php from PHP source
25-
RUN wget -q -O /opt/run-tests.php https://raw.githubusercontent.com/php/php-src/master/run-tests.php
88+
# Enable extension
89+
RUN echo "extension=signalforge_http.so" > /usr/local/etc/php/conf.d/signalforge_http.ini
2690

2791
# Configure PHP for Valgrind
2892
RUN echo "zend.assertions=1" >> /usr/local/etc/php/conf.d/valgrind.ini
2993

94+
# Get run-tests.php from PHP source
95+
RUN wget -q -O /opt/run-tests.php https://raw.githubusercontent.com/php/php-src/master/run-tests.php
96+
3097
# Verify extension is loaded
3198
RUN php -m | grep signalforge_http
3299

‎Makefile‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,37 @@
11
# Signalforge HTTP Extension - Docker-based Build
22
IMAGE_NAME = signalforge-http
3-
VERSION ?= 85
3+
PHP_VERSION ?= 8.5
44

55
.PHONY: docker-build docker-test docker-example docker-shell docker-clean valgrind-test valgrind-docker ci-test-all test-version build-push help
66

77
help:
88
@echo "Signalforge HTTP Extension"
99
@echo ""
1010
@echo "Usage:"
11-
@echo " make docker-build - Build Docker image with extension (default: PHP 8.5, VERSION=85)"
12-
@echo " make docker-test - Run tests in Docker (default: PHP 8.5, VERSION=85)"
11+
@echo " make docker-build - Build Docker image with extension (default: PHP 8.5)"
12+
@echo " make docker-test - Run tests in Docker (default: PHP 8.5)"
1313
@echo " make docker-example - Run example in Docker"
1414
@echo " make docker-shell - Interactive shell in Docker"
1515
@echo " make docker-clean - Remove Docker images"
1616
@echo " make ci-test-all - Build and test PHP 8.3, 8.4, and 8.5"
17-
@echo " make test-version - Run tests using ghcr.io image (VERSION=85)"
17+
@echo " make test-version - Run tests using locally built extension"
1818
@echo " make valgrind-docker - Run Valgrind memory check in Docker"
1919
@echo " make valgrind-test - Run tests with local Valgrind"
2020
@echo ""
21-
@echo "Supported versions: VERSION=83 (PHP 8.3), VERSION=84 (PHP 8.4), VERSION=85 (PHP 8.5)"
22-
@echo "Example: make docker-build VERSION=84"
21+
@echo "Supported versions: PHP_VERSION=8.3, PHP_VERSION=8.4, PHP_VERSION=8.5 (default)"
22+
@echo "Example: make docker-build PHP_VERSION=8.4"
2323

2424
docker-build:
25-
docker build --build-arg VERSION=$(VERSION) -t $(IMAGE_NAME):$(VERSION) -t $(IMAGE_NAME):latest .
25+
docker build --platform linux/amd64 --build-arg PHP_VERSION=$(PHP_VERSION) -t $(IMAGE_NAME):$(PHP_VERSION) -t $(IMAGE_NAME):latest .
2626

2727
docker-test:
28-
docker run --rm -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):$(VERSION) php /opt/run-tests.php /ext/tests/
28+
docker run --rm --platform linux/amd64 -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):$(PHP_VERSION) php /opt/run-tests.php /ext/tests/
2929

3030
docker-example:
31-
docker run --rm -v $(PWD)/examples:/ext/examples $(IMAGE_NAME):$(VERSION) php /ext/examples/basic.php
31+
docker run --rm --platform linux/amd64 -v $(PWD)/examples:/ext/examples $(IMAGE_NAME):$(PHP_VERSION) php /ext/examples/basic.php
3232

3333
docker-shell:
34-
docker run --rm -it -v $(PWD):/ext $(IMAGE_NAME):$(VERSION) sh
34+
docker run --rm --platform linux/amd64 -it -v $(PWD):/ext $(IMAGE_NAME):$(PHP_VERSION) bash
3535

3636
docker-clean:
3737
docker rmi $(IMAGE_NAME) 2>/dev/null || true
@@ -55,33 +55,33 @@ valgrind-test:
5555
# CI: Build and test all PHP versions
5656
ci-test-all:
5757
@echo "Building and testing PHP 8.3..."
58-
docker build --build-arg VERSION=83 -t $(IMAGE_NAME):83 .
59-
docker run --rm -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):83 php /opt/run-tests.php -q /ext/tests/
58+
docker build --platform linux/amd64 --build-arg PHP_VERSION=8.3 -t $(IMAGE_NAME):8.3 .
59+
docker run --rm --platform linux/amd64 -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):8.3 php /opt/run-tests.php -q /ext/tests/
6060
@echo ""
6161
@echo "Building and testing PHP 8.4..."
62-
docker build --build-arg VERSION=84 -t $(IMAGE_NAME):84 .
63-
docker run --rm -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):84 php /opt/run-tests.php -q /ext/tests/
62+
docker build --platform linux/amd64 --build-arg PHP_VERSION=8.4 -t $(IMAGE_NAME):8.4 .
63+
docker run --rm --platform linux/amd64 -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):8.4 php /opt/run-tests.php -q /ext/tests/
6464
@echo ""
6565
@echo "Building and testing PHP 8.5..."
66-
docker build --build-arg VERSION=85 -t $(IMAGE_NAME):85 .
67-
docker run --rm -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):85 php /opt/run-tests.php -q /ext/tests/
66+
docker build --platform linux/amd64 --build-arg PHP_VERSION=8.5 -t $(IMAGE_NAME):8.5 .
67+
docker run --rm --platform linux/amd64 -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):8.5 php /opt/run-tests.php -q /ext/tests/
6868
@echo ""
6969
@echo "All PHP versions tested successfully!"
7070

7171
# Valgrind: Run memory check in Docker
7272
valgrind-docker:
7373
@echo "Building Valgrind Docker image..."
74-
docker build -f Dockerfile.valgrind -t $(IMAGE_NAME):valgrind .
74+
docker build --platform linux/amd64 --build-arg PHP_VERSION=$(PHP_VERSION) -f Dockerfile.valgrind -t $(IMAGE_NAME):valgrind .
7575
@echo ""
7676
@echo "Running Valgrind memory check..."
77-
docker run --rm -v $(PWD)/tests:/ext/tests -v $(PWD):/output $(IMAGE_NAME):valgrind \
78-
sh -c 'valgrind --leak-check=full --show-leak-kinds=all --error-exitcode=1 \
77+
docker run --rm --platform linux/amd64 -v $(PWD)/tests:/ext/tests -v $(PWD):/output $(IMAGE_NAME):valgrind \
78+
bash -c 'valgrind --leak-check=full --show-leak-kinds=all --error-exitcode=1 \
7979
--track-origins=yes --log-file=/output/valgrind-output.txt \
8080
php /opt/run-tests.php -q /ext/tests/ && \
8181
cat /output/valgrind-output.txt | grep -E "ERROR SUMMARY|LEAK SUMMARY" -A 5'
8282

83-
# Run tests using pre-built ghcr.io image with configurable PHP version
83+
# Run tests using locally built extension with configurable PHP version
8484
test-version:
85-
@echo "Running tests with PHP 8.$(VERSION) from ghcr.io..."
86-
docker run --rm -v $(PWD):/ext ghcr.io/thesignalforge/signalforge:php$(VERSION) \
87-
php /usr/local/lib/php/build/run-tests.php /ext/tests/
85+
@echo "Running tests with locally built PHP $(PHP_VERSION) extension (linux/amd64)..."
86+
docker run --rm --platform linux/amd64 -v $(PWD)/tests:/ext/tests $(IMAGE_NAME):$(PHP_VERSION) \
87+
php /opt/run-tests.php /ext/tests/

‎src/request.c‎

Lines changed: 29 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -831,56 +831,6 @@ PHP_METHOD(Signalforge_Http_Request, create)
831831
RETURN_THROWS();
832832
}
833833

834-
/* Extract URI string from parameter */
835-
if (Z_TYPE_P(uri_param) == IS_STRING) {
836-
uri_str = Z_STR_P(uri_param);
837-
} else if (Z_TYPE_P(uri_param) == IS_OBJECT && instanceof_function(Z_OBJCE_P(uri_param), signalforge_uri_ce)) {
838-
/* Uri object - convert to string */
839-
signalforge_uri_object *uri_obj = Z_SIGNALFORGE_URI_P(uri_param);
840-
smart_str buf = {0};
841-
842-
if (uri_obj->scheme && ZSTR_LEN(uri_obj->scheme) > 0) {
843-
smart_str_append(&buf, uri_obj->scheme);
844-
smart_str_appendl(&buf, "://", 3);
845-
}
846-
if (uri_obj->host && ZSTR_LEN(uri_obj->host) > 0) {
847-
if (uri_obj->user && ZSTR_LEN(uri_obj->user) > 0) {
848-
smart_str_append(&buf, uri_obj->user);
849-
if (uri_obj->pass && ZSTR_LEN(uri_obj->pass) > 0) {
850-
smart_str_appendc(&buf, ':');
851-
smart_str_append(&buf, uri_obj->pass);
852-
}
853-
smart_str_appendc(&buf, '@');
854-
}
855-
smart_str_append(&buf, uri_obj->host);
856-
if (uri_obj->port != SIGNALFORGE_PORT_UNSET &&
857-
!signalforge_is_standard_port(uri_obj->scheme, uri_obj->port)) {
858-
smart_str_appendc(&buf, ':');
859-
smart_str_append_long(&buf, uri_obj->port);
860-
}
861-
}
862-
if (uri_obj->path && ZSTR_LEN(uri_obj->path) > 0) {
863-
smart_str_append(&buf, uri_obj->path);
864-
} else if (!uri_obj->host || ZSTR_LEN(uri_obj->host) == 0) {
865-
smart_str_appendc(&buf, '/');
866-
}
867-
if (uri_obj->query && ZSTR_LEN(uri_obj->query) > 0) {
868-
smart_str_appendc(&buf, '?');
869-
smart_str_append(&buf, uri_obj->query);
870-
}
871-
if (uri_obj->fragment && ZSTR_LEN(uri_obj->fragment) > 0) {
872-
smart_str_appendc(&buf, '#');
873-
smart_str_append(&buf, uri_obj->fragment);
874-
}
875-
876-
smart_str_0(&buf);
877-
uri_str = buf.s ? buf.s : zend_empty_string;
878-
} else {
879-
zend_throw_exception(spl_ce_InvalidArgumentException,
880-
"URI must be a string or Uri object", 0);
881-
RETURN_THROWS();
882-
}
883-
884834
/* Create new instance */
885835
object_init_ex(return_value, signalforge_request_ce);
886836
intern = Z_SIGNALFORGE_REQUEST_P(return_value);
@@ -902,15 +852,37 @@ PHP_METHOD(Signalforge_Http_Request, create)
902852
ALLOC_HASHTABLE(intern->ht_attributes);
903853
zend_hash_init(intern->ht_attributes, 8, NULL, ZVAL_PTR_DTOR, 0);
904854

905-
/* Set the URI */
906-
ZVAL_STR_COPY(&intern->zv_uri, uri_str);
907-
intern->request_uri = Z_STRVAL(intern->zv_uri);
908-
intern->request_uri_len = Z_STRLEN(intern->zv_uri);
855+
/* Set the URI - handle ownership correctly */
856+
if (Z_TYPE_P(uri_param) == IS_STRING) {
857+
/* Borrowed from parameter - must copy */
858+
ZVAL_STR_COPY(&intern->zv_uri, Z_STR_P(uri_param));
859+
} else if (Z_TYPE_P(uri_param) == IS_OBJECT && instanceof_function(Z_OBJCE_P(uri_param), signalforge_uri_ce)) {
860+
/* Uri object - call __toString() and transfer ownership */
861+
zval uri_string_zv;
862+
zend_call_method_with_0_params(Z_OBJ_P(uri_param), Z_OBJCE_P(uri_param),
863+
NULL, "__tostring", &uri_string_zv);
864+
865+
if (EG(exception)) {
866+
RETURN_THROWS();
867+
}
868+
869+
if (Z_TYPE(uri_string_zv) != IS_STRING) {
870+
zval_ptr_dtor(&uri_string_zv);
871+
zend_throw_exception(spl_ce_RuntimeException,
872+
"Uri::__toString() must return a string", 0);
873+
RETURN_THROWS();
874+
}
909875

910-
/* Release temporary uri_str if it was created from Uri object conversion */
911-
if (Z_TYPE_P(uri_param) == IS_OBJECT && instanceof_function(Z_OBJCE_P(uri_param), signalforge_uri_ce)) {
912-
zend_string_release(uri_str);
876+
/* Transfer ownership from temporary zval to object */
877+
ZVAL_COPY_VALUE(&intern->zv_uri, &uri_string_zv);
878+
ZVAL_UNDEF(&uri_string_zv); /* Mark as transferred, no double-free */
879+
} else {
880+
zend_throw_exception(spl_ce_InvalidArgumentException,
881+
"URI must be a string or Uri object", 0);
882+
RETURN_THROWS();
913883
}
884+
intern->request_uri = Z_STRVAL(intern->zv_uri);
885+
intern->request_uri_len = Z_STRLEN(intern->zv_uri);
914886

915887
/* Set query string if present in URI */
916888
const char *query_pos = strchr(Z_STRVAL(intern->zv_uri), '?');

‎src/response.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,9 +636,14 @@ PHP_METHOD(Signalforge_Http_Response, getHeaderLine)
636636
} else if (Z_TYPE_P(val) == IS_STRING) {
637637
smart_str_appendl(&str, Z_STRVAL_P(val), Z_STRLEN_P(val));
638638
}
639-
639+
640640
smart_str_0(&str);
641-
RETURN_STR(str.s);
641+
/* Handle edge case where header exists but is empty */
642+
if (str.s) {
643+
RETURN_STR(str.s);
644+
} else {
645+
RETURN_EMPTY_STRING();
646+
}
642647
}
643648
/* }}} */
644649

0 commit comments

Comments
 (0)