Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

fix(WhoopsError): ensure the SCRIPT_NAME is set before attempting to use it #489

Merged
merged 2 commits into from
Oct 9, 2017

Conversation

gsdevme
Copy link
Contributor

@gsdevme gsdevme commented May 26, 2017

PSR states

     * Retrieves data related to the incoming request environment,
     * typically derived from PHP's $_SERVER superglobal. The data IS NOT
     * REQUIRED to originate from $_SERVER.

Nothing suggests that SCRIPT_NAME will always exist. Therefore I've added a isset() guard and left the default as an empty string.


if (isset($serverParams['SCRIPT_NAME'])) {
$scriptName = $serverParams['SCRIPT_NAME'];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be cleaned up to:

$serverParams = $request->getServerParams();
$scriptName = (isset($serverParams['SCRIPT_NAME'])) ? $serverParams['SCRIPT_NAME'] : '';

It'll be nice when ZE is pinned to PHP 7.1 and we can use null coalesce operators...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Always found it bizarre that people think this is "cleaner". I didn't realise I had written really unwieldily code here. I guess the above is also using a ternary operator so it does at least make it consistent.

@weierophinney weierophinney added this to the 2.0.4 milestone Oct 9, 2017
@weierophinney weierophinney merged commit 3bc5640 into zendframework:master Oct 9, 2017
weierophinney added a commit that referenced this pull request Oct 9, 2017
fix(WhoopsError): ensure the SCRIPT_NAME is set before attempting to use it
weierophinney added a commit that referenced this pull request Oct 9, 2017
weierophinney added a commit that referenced this pull request Oct 9, 2017
weierophinney added a commit that referenced this pull request Oct 9, 2017
Forward port #489

Conflicts:
	CHANGELOG.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants