Skip to content

Type checks in use statements and parameter type hints - #10

Open
LukasRos wants to merge 1 commit into
Corveda:mainfrom
LukasRos:master
Open

Type checks in use statements and parameter type hints#10
LukasRos wants to merge 1 commit into
Corveda:mainfrom
LukasRos:master

Conversation

@LukasRos

Copy link
Copy Markdown

I've extended the code to check for whitelisted types and interfaces in parameter type hints and use statements. Even though this is not strictly required for practical sandbox constraints; for my use of this library this is very helpful. I'm not sure if this breaks other use cases; if so I can add a switch.

Comment thread src/ValidatorVisitor.php
$node->type = new Node\Name($this->sandbox->getDefinedClass($class));
}
if ($this->sandbox->isWhitelistedInterface($class))
$this->sandbox->checkInterface($class);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CS :s

Comment thread src/ValidatorVisitor.php
} else {
$this->sandbox->validationError("Sandboxed code attempted use invalid namespace or alias!", Error::DEFINE_ALIAS_ERROR, $node);
}
if ($this->sandbox->isWhitelistedInterface($use->alias))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CS :s

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry, what does "CS :s" mean?
Also, I think I still have some flawed logic in this, I will work on this again and update the PR later.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"Coding style": if ($this->sandbox->isWhitelistedInterface($use->alias)) { ;)

* Test whether sandbox disallows non-whitelisted classes in parameter type hints
*/
public function testDisallowsTypeInParam(){
$this->expectException('PHPSandbox\Error');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sur, but use statement and Error::class ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In this test I'm expecting the use statement to fail even though use statements are allowed because the class I'm using is not whitelisted.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I mean:
in top of file:
use PHPSandbox\Error;

And line 448:
$this->expectException(Error::class);

@peter279k

Copy link
Copy Markdown
Contributor

The tests/DefaultConfigTest.php file has been conflicts and it should be concerned.

@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@squash-labs

squash-labs Bot commented Jan 24, 2023

Copy link
Copy Markdown

Manage this branch in Squash

Test this branch here: https://master-63c39.squash.io

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