Skip to content
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

Engine: Make second parameter mandatory (Fix open_basedir restriction) #5

Merged
merged 1 commit into from
Mar 12, 2018
Merged

Conversation

RiKap
Copy link
Contributor

@RiKap RiKap commented Mar 12, 2018

Hi @Machy8
If server is using open_basedir restrictions it is not working.
Main problem is default value of $documentRoot in Engine. Because it is root of server. Function setDocumentRoot() in Compiler will try check if given value / is directory, but it fails because of open_basedir restrictions.

After these changes it is OK. But it is BC break because $documentRoot is now mandatory.

@coveralls
Copy link

coveralls commented Mar 12, 2018

Coverage Status

Coverage remained the same at 93.837% when pulling 57c6673 on RiKap:patch-1 into 7f94500 on Machy8:master.

Copy link
Owner

@Machy8 Machy8 left a comment

Choose a reason for hiding this comment

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

CR done.

@@ -91,16 +91,12 @@ private function setupWebLoader()
{
$webLoader = $this->builder->addDefinition($this->prefix(self::ENGINE_PREFIX))
->setFactory(self::ENGINE_CLASSNAME)
->setArguments([$this->config['outputDir']]);
->setArguments([$this->config['outputDir'], $this->config['documentRoot']]);
Copy link
Owner

Choose a reason for hiding this comment

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

Thank you for the pull request!

I think that this change don't need to be a BC break. If the condition from below would be used here, then the parameter could still be optional. Could you please try it on your project where it cause problems? If it works, then the other changes could be reverted.

$arguments = [$this->config['outputDir']];

if ($this->config['documentRoot']) {
    $arguments[] = $this->config['documentRoot'];
}

$webLoader = $this->builder->addDefinition($this->prefix(self::ENGINE_PREFIX))
    ->setFactory(self::ENGINE_CLASSNAME)
    ->setArguments($arguments);

@RiKap
Copy link
Contributor Author

RiKap commented Mar 12, 2018

@Machy8 Done without BC break.
If it is OK for you, please release new version. Thank you.

@Machy8 Machy8 merged commit f0e866f into Machy8:master Mar 12, 2018
@Machy8
Copy link
Owner

Machy8 commented Mar 12, 2018

@RiKap RiKap deleted the patch-1 branch March 12, 2018 17:32
@RiKap RiKap changed the title Engine: Make mandatory (Fix open_basedir restriction) Engine: Make second parameter mandatory (Fix open_basedir restriction) Mar 15, 2018
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.

3 participants