Skip to content

USE $_SERVER['SCRIPT_FILENAME'] instead of getenv() - #5720

Open
Caprico85 wants to merge 1 commit into
OpenMage:mainfrom
Caprico85:env-script-filename
Open

USE $_SERVER['SCRIPT_FILENAME'] instead of getenv()#5720
Caprico85 wants to merge 1 commit into
OpenMage:mainfrom
Caprico85:env-script-filename

Conversation

@Caprico85

Copy link
Copy Markdown
Contributor

Description (*)

This change replaces getenv('SCRIPT_FILENAME') with $_SERVER['SCRIPT_FILENAME'].

Otherwise, if you try to access the REST API using FrankenPHP as the web server, you will receive error 400: 'API type "api" is not supported'.

<magento_api>
	<script/>
	<messages>
		<error>
			<data_item>
				<code>400</code>
				<message>API type "api" is not supported</message>
				<trace>#0 /xxx/app/code/core/Mage/Api2/Model/Server.php(190): Mage_Api2_Model_Config->getRoutes('api') #1 /var/www/xxx/app/code/core/Mage/Api2/Model/Server.php(96): Mage_Api2_Model_Server->_route(Object(Mage_Api2_Model_Request)) #2 /xxx/api.php(47): Mage_Api2_Model_Server->run() #3 /var/www/xxx/pub/default/api.php(7): require('/xxx...') #4 {main}</trace>
			</data_item>
		</error>
	</messages>
</magento_api>

In app/code/core/Mage/Api2/Model/Route/ApiType.php, OpenMage uses getenv('SCRIPT_FILENAME') to read the script filename from an environment variable.

This works with "classic" webservers, such as Apache or nginx and the PHP-FPM runtime. These servers set variables such as SCRIPT_FILENAME as both environment variables and in the $_SERVER superglobal. More modern PHP runtimes, such as FrankenPHP only expose these variables in the $_SERVER superglobal and not as environment variables. As far as i understand, this is done for better isolation in FrankenPHP's worker mode. An environment variable would be visible to all PHP requests within the same worker process, whereas the $_SERVER superglobal is set individually set for each request.

Therefore, getenv('SCRIPT_FILENAME') can return incorrect or empty values on these modern runtimes. It's safer to use $_SERVER['SCRIPT_FILENAME'], which works on all runtimes (including Apache, nginx/FPM, Caddy/FPM, and FrankenPHP).

Manual testing scenarios (*)

  1. Run a store using FrankenPHP as the web server
  2. Open /api/rest/orders in your browser (or any REST client, but a browser will suffice)
  3. You will receive a 400 "API type "api" is not supported". It is expected to get a 401 Unauthorized or 403 Access denied, because you are not sending access tokens.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All automated tests passed successfully (all builds are green, SonarCloud checks are not required to merge)

@Caprico85

Caprico85 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Okay, I didn't expect the PHPMD error. Let me find another way.
Edit: Suppressed the warnings using @SuppressWarnings("PHPMD.Superglobals"). I hope it's okay in this case.

@Caprico85
Caprico85 marked this pull request as draft August 4, 2026 11:45
@Caprico85
Caprico85 force-pushed the env-script-filename branch 2 times, most recently from e6a353e to e5a054a Compare August 4, 2026 12:11
@Caprico85
Caprico85 marked this pull request as ready for review August 4, 2026 12:31
@Hanmac
Hanmac requested review from addison74 and sreichel August 4, 2026 13:04
Hanmac
Hanmac previously approved these changes Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves API routing and URL generation by replacing getenv('SCRIPT_FILENAME') with $_SERVER['SCRIPT_FILENAME'], addressing cases where modern runtimes (e.g., FrankenPHP worker mode) do not expose per-request values via environment variables.

Changes:

  • Use $_SERVER['SCRIPT_FILENAME'] to derive the API2 route prefix (avoids getenv() returning empty/incorrect values).
  • Use $_SERVER['SCRIPT_FILENAME'] when building SOAP service URLs in Mage_Api_Helper_Data.
  • Remove now-unneeded PHPStan baseline ignores related to basename(getenv(...)) potentially returning false.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
app/code/core/Mage/Api2/Model/Route/ApiType.php Switches API type route prefix derivation from getenv() to $_SERVER.
app/code/core/Mage/Api/Helper/Data.php Switches script name derivation for service URL path building from getenv() to $_SERVER.
.phpstan.dist.baselines/argument.type.php Removes baseline entries that are no longer applicable after the change.

Comment thread app/code/core/Mage/Api/Helper/Data.php Outdated
Comment thread app/code/core/Mage/Api2/Model/Route/ApiType.php Outdated
sreichel
sreichel previously approved these changes Aug 6, 2026
@Caprico85
Caprico85 dismissed stale reviews from sreichel and Hanmac via 25479b8 August 7, 2026 07:53
@Caprico85
Caprico85 force-pushed the env-script-filename branch 2 times, most recently from 25479b8 to 5f85a95 Compare August 7, 2026 07:54
@Caprico85

Copy link
Copy Markdown
Contributor Author

I added the changes suggested by Copilot.

@Caprico85
Caprico85 force-pushed the env-script-filename branch from 5f85a95 to 86f88f0 Compare August 7, 2026 09:35
@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@Caprico85

Copy link
Copy Markdown
Contributor Author

Fixed the rector warning too

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants