Skip to content

PHP : Set appropriate SCRIPT variables in $_SERVER superglobal - #1092

Merged
adamziel merged 3 commits into
WordPress:trunkfrom
mho22:appropriate-script-name-and-filename-in-server-superglobal-patch
Mar 7, 2024
Merged

PHP : Set appropriate SCRIPT variables in $_SERVER superglobal#1092
adamziel merged 3 commits into
WordPress:trunkfrom
mho22:appropriate-script-name-and-filename-in-server-superglobal-patch

Conversation

@mho22

@mho22 mho22 commented Mar 7, 2024

Copy link
Copy Markdown
Collaborator

What is this PR doing?

Based on the issue #1090

When requesting a url, the current behavior is copying the value from REQUEST_URI to SCRIPT_NAME and SCRIPT_FILENAME $_SERVER variables.

It should actually be related to the PHP_SELF variable when handling request instead of REQUEST_URI when running code.

What problem is it solving?

To avoid a mismatch when using SCRIPT variables

Testing Instructions

const php = await NodePHP.load( '8.2, { requestHandler : { documentRoot : '/php' } } );

php.writeFile( 'php/index.php', "<?php echo json_encode($_SERVER);" );

const response = await php.request({
	url: '/',
	method: 'GET',
});

console.log( JSON.stringify( response.json, null, 2 );

This should display :

REQUEST_URI='/'
SCRIPT_NAME='/index.php'
SCRIPT_FILENAME='/php/index.php'
PHP_SELF='/index.php'

instead of currently :

REQUEST_URI='/'
SCRIPT_NAME='/'
SCRIPT_FILENAME='/'
PHP_SELF='/'
const json = response.json;

expect(json).toHaveProperty('REQUEST_URI', '/');
expect(json).toHaveProperty('SCRIPT_NAME', '/index.php');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also add a test to see what happens when the file is called test.php is in a subdirectory, e.g. /php/subdirectory/test.php

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamziel I added the test you asked. Is it what you expected ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thank you!

@adamziel
adamziel merged commit ff2727e into WordPress:trunk Mar 7, 2024
@mho22
mho22 deleted the appropriate-script-name-and-filename-in-server-superglobal-patch branch March 8, 2024 11:44
@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Feature] PHP.wasm labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] PHP.wasm [Type] Bug An existing feature does not function as intended

2 participants