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

ContentSecurityPolicy::setReportURI() shoud accept null values #9091

Closed
allpassos opened this issue Jul 30, 2024 · 3 comments · Fixed by #9100
Closed

ContentSecurityPolicy::setReportURI() shoud accept null values #9091

allpassos opened this issue Jul 30, 2024 · 3 comments · Fixed by #9100

Comments

@allpassos
Copy link

allpassos commented Jul 30, 2024

PHP Version

8.1

CodeIgniter4 Version

4.5.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MySQL 8.0.30

What happened?

Trying to set with runntime $reportURI property from app/Config/ContentSecurityPolicy.php to null in BaseController, throws an exception:

message: CodeIgniter\HTTP\ContentSecurityPolicy::setReportURI(): Argument #1 ($uri) must be of type string, null given, called in C:\laragon\www\...\app\Controllers\BaseController.php on line 70
file: C:\laragon\www\newawm\vendor\codeigniter4\framework\system\HTTP\ContentSecurityPolicy.php  on line 599

This shoud happen because $reportURI from app/Config/ContentSecurityPolicy.php accept ?string. The sabe the core config file. However the propertie inside system\HTTP\ContentSecurityPolicy.php just accept strings.

__

Steps to Reproduce

  1. Inside BaseController -> public function initController add the following code:
if(!empty($this->session->get('system'))) { ...
   // get the CSP instance
   $csp = $this->response->getCSP();
   $csp->setReportURI(null);

Expected Output

The CSP (from Codeigniter\HTTP\ContentSecurityPolicy) object inside the response object (Codeigniter\HTTP\Response) setted as null

Anything else?

$csp->setReportURI('null') or $csp->setReportURI('') do the job. So I can handle it in the model

@allpassos allpassos added the bug Verified issues on the current code behavior or pull requests that will fix them label Jul 30, 2024
@kenjis
Copy link
Member

kenjis commented Jul 30, 2024

Do you set the default $reportURI in the config file, and want to remove it at run time?
Why do you need to do so?

It seems $csp->setReportURI('') should work as expected.

@kenjis kenjis changed the title Bug: protected $reportURI property from \CodeIgniter\HTTP\ContentSecurityPolicy shoud accept null values Bug: ContentSecurityPolicy::setReportURI() shoud accept null values Jul 30, 2024
@kenjis kenjis added the waiting for info Issues or pull requests that need further clarification from the author label Jul 31, 2024
@allpassos
Copy link
Author

Hello kenjis.
I did set the $reportURI in the config file. Eje.: /csp-report-uri/store. This URL receive the post request from and save it into DB. It is the default behavior/value for my system CSP.
In the other hand i have an admin panel where i can change one variable and change the system csp-report-uri behavior deleting from the REQUEST header that .
So i've the control of the repoting thing

@kenjis kenjis removed the bug Verified issues on the current code behavior or pull requests that will fix them label Aug 2, 2024
@kenjis
Copy link
Member

kenjis commented Aug 2, 2024

If you want to change Config settings by an admin panel, we have https://github.com/codeigniter4/settings

In any case, you can remove the report-uri directive with $csp->setReportURI('') at runtime.
I sent a PR to add PHPDoc:
https://github.com/codeigniter4/CodeIgniter4/pull/9100/files#diff-aaf75ce6094ff1dfaddf13c9b2f8c9e587e725d5413d392ee783069760bd34b7R598-R600

If we change the setReportURI() to accept null, it will be a breaking change.
I don't think it is worth doing it.

@kenjis kenjis removed the waiting for info Issues or pull requests that need further clarification from the author label Aug 2, 2024
@kenjis kenjis changed the title Bug: ContentSecurityPolicy::setReportURI() shoud accept null values ContentSecurityPolicy::setReportURI() shoud accept null values Aug 2, 2024
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 a pull request may close this issue.

2 participants