davidcole1340 / ext-php-rs

Bindings for the Zend API to build PHP extensions natively in Rust.
Apache License 2.0
585 stars 63 forks source link

`TRACK_VARS_REQUEST` seems to not be stored in `http_globals` #331

Open Xenira opened 5 hours ago

Xenira commented 5 hours ago

Rust nightly build detects that the access in https://github.com/davidcole1340/ext-php-rs/blob/0d9496b76cce8914469b60f90983283c2fdb0db0/src/zend/globals.rs#L287 results in a out of bounds.

Looking at the C code it seems that TRACK_VARS_REQUEST is not referenced anywhere.

Maybe someone with more knowledge about php internals can point towards where to find the $_REQUEST super global.

ptondereau commented 5 hours ago

Very weird as this constant lives here: https://github.com/php/php-src/blob/master/main/php_globals.h#L46

Xenira commented 5 hours ago

Ye, found the global both in the bindings file and in the zend code https://github.com/php/php-src/blob/50acf5eb15df4be623e01af7926688ce8fc7529b/main/php_globals.h#L46

But looking at the array size (https://github.com/php/php-src/blob/50acf5eb15df4be623e01af7926688ce8fc7529b/main/php_globals.h#L114) it is 6 and therefore out of bounds with index 6.

Searching for TRACK_VARS_REQUEST does not yield anything but the definition.

ptondereau commented 5 hours ago

Ok, but it seems that it's related to Rust according to this issue: https://github.com/rust-lang/rust/issues/90534

Xenira commented 5 hours ago

Sure, might be my limited c knowledge, but shouldn't the http_globals be constant length?

ptondereau commented 5 hours ago

So as we know that the 6th index exists, we could tell the compiler to disable this warning on this specific line instead

Xenira commented 5 hours ago

Just created a simple test:

#![cfg_attr(windows, feature(abi_vectorcall))]
use ext_php_rs::prelude::*;
use ext_php_rs::zend::ProcessGlobals;

#[php_function]
pub fn hello_world(name: &str) -> String {
    format!("Hello, {:?}!", ProcessGlobals::get().http_request_vars())
}

#[php_module]
pub fn get_module(module: ModuleBuilder) -> ModuleBuilder {
    module
}

Results in

thread '<unnamed>' panicked at /home/rippi/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ext-php-rs-0.12.0/src/zend/globals.rs:287:9:
index out of bounds: the len is 6 but the index is 6
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
zsh: IOT instruction (core dumped)  php -d extension=./target/debug/libreq_super_global.so test.php
ptondereau commented 5 hours ago

Ok thank you for your test! I've created an issue on php-src repo because IMHO, it's a bug from PHP here https://github.com/php/php-src/issues/16541

Xenira commented 2 hours ago

Managed to get somewhere. Still only getting a null value, but making progress.

Xenira commented 2 hours ago

Hmmm, from what I can see this might not be initialised all the time and needs some priming. Logging the symbol table I get

{
    "_GET": Zval {
        type: Array,
        val: Some(
            {},
        ),
    },
    "_POST": Zval {
        type: Array,
        val: Some(
            {},
        ),
    },
    "_COOKIE": Zval {
        type: Array,
        val: Some(
            {},
        ),
    },
    "_FILES": Zval {
        type: Array,
        val: Some(
            {},
        ),
    },
}

which does not include the _REQUEST entry, causing zend_hash_find_* to return a nullpointer.

Gonna do some more testing tomorrow. Pushed my experiments for reference.