Skip to content

Fix SAPI shutdown #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ console.log(response.body.toString())
### `new Php(config)`

* `config` {Object} Configuration object
* `argv` {String[]} Process arguments. **Default:** []
* `docroot` {String} Document root for PHP. **Default:** process.cwd()
* Returns: {Php}

Expand All @@ -67,6 +68,7 @@ Construct a new PHP instance to which to dispatch requests.
import { Php } from '@platformatic/php-node'

const php = new Php({
argv: process.argv,
docroot: process.cwd()
})
````
Expand Down
39 changes: 31 additions & 8 deletions __test__/handler.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ test('Support input/output streams', async (t) => {
t.teardown(() => mockroot.clean())

const php = new Php({
argv: process.argv,
docroot: mockroot.path
})

const req = new Request({
method: 'GET',
method: 'POST',
url: 'http://example.com/index.php',
body: Buffer.from('Hello, from Node.js!')
})
Expand All @@ -39,12 +38,10 @@ test('Capture logs', async (t) => {
t.teardown(() => mockroot.clean())

const php = new Php({
argv: process.argv,
docroot: mockroot.path
})

const req = new Request({
method: 'GET',
url: 'http://example.com/index.php'
})

Expand All @@ -62,12 +59,10 @@ test('Capture exceptions', async (t) => {
t.teardown(() => mockroot.clean())

const php = new Php({
argv: process.argv,
docroot: mockroot.path
})

const req = new Request({
method: 'GET',
url: 'http://example.com/index.php'
})

Expand All @@ -89,12 +84,10 @@ test('Support request and response headers', async (t) => {
t.teardown(() => mockroot.clean())

const php = new Php({
argv: process.argv,
docroot: mockroot.path
})

const req = new Request({
method: 'GET',
url: 'http://example.com/index.php',
headers: {
'X-Test': ['Hello, from Node.js!']
Expand All @@ -106,3 +99,33 @@ test('Support request and response headers', async (t) => {
t.is(res.body.toString(), 'Hello, from Node.js!')
t.is(res.headers.get('X-Test'), 'Hello, from PHP!')
})

test('Has expected args', async (t) => {
const mockroot = await MockRoot.from({
'index.php': `<?php
echo "[";
$first = true;
foreach ($argv as $value) {
if ($first) { $first = false; }
else { echo ","; }
echo "\\"$value\\"";
}
echo "]";
?>`
})
t.teardown(() => mockroot.clean())

const php = new Php({
argv: process.argv,
docroot: mockroot.path
})

const req = new Request({
url: 'http://example.com/index.php'
})

const res = await php.handleRequest(req)
t.is(res.status, 200)

t.is(res.body.toString('utf8'), JSON.stringify(process.argv))
})
37 changes: 26 additions & 11 deletions crates/php/src/embed.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::{
env::Args,
ffi::c_char,
ffi::{c_char, CString},
path::{Path, PathBuf},
sync::Arc,
};

use ext_php_rs::{
Expand All @@ -25,6 +26,14 @@ use crate::{
#[derive(Debug)]
pub struct Embed {
docroot: PathBuf,

// TODO: Do something with this...
#[allow(dead_code)]
args: Vec<String>,

// NOTE: This needs to hold the SAPI to keep it alive
#[allow(dead_code)]
sapi: Arc<Sapi>,
}

// An embed instance may be constructed on the main thread and then shared
Expand Down Expand Up @@ -90,14 +99,16 @@ impl Embed {
C: AsRef<Path>,
S: AsRef<str> + std::fmt::Debug,
{
ensure_sapi(argv)?;

let docroot = docroot
.as_ref()
.canonicalize()
.map_err(|_| EmbedException::DocRootNotFound(docroot.as_ref().display().to_string()))?;

Ok(Embed { docroot })
Ok(Embed {
docroot,
args: argv.iter().map(|v| v.as_ref().to_string()).collect(),
sapi: ensure_sapi()?,
})
}

/// Get the docroot used for this Embed instance
Expand Down Expand Up @@ -152,12 +163,8 @@ impl Handler for Embed {
/// //assert_eq!(response.body(), "Hello, world!");
/// ```
fn handle(&self, request: Request) -> Result<Response, Self::Error> {
unsafe {
ext_php_rs::embed::ext_php_rs_sapi_per_thread_init();
}

// Initialize the SAPI module
Sapi::startup()?;
self.sapi.startup()?;

let url = request.url();

Expand All @@ -182,6 +189,14 @@ impl Handler for Embed {
.unwrap_or(0);
let cookie_data = nullable_cstr(headers.get("Cookie"))?;

let argc = self.args.len() as i32;
let mut argv_ptrs = vec![];
for arg in self.args.iter() {
let string = CString::new(arg.as_bytes())
.map_err(|_| EmbedException::CStringEncodeFailed(arg.to_owned()))?;
argv_ptrs.push(string.into_raw());
}

// Prepare memory stream of the code
let mut file_handle = unsafe {
let mut file_handle = zend_file_handle {
Expand Down Expand Up @@ -214,8 +229,8 @@ impl Handler for Embed {

// Reset state
globals.request_info.proto_num = 110;
globals.request_info.argc = 0;
globals.request_info.argv = std::ptr::null_mut();
globals.request_info.argc = argc;
globals.request_info.argv = argv_ptrs.as_mut_ptr();
globals.request_info.headers_read = false;
globals.sapi_headers.http_response_code = 200;

Expand Down
Loading
Loading