Problem/Motivation

The log is flooded with the same error every time a page gets loaded:
Notice: Uninitialized string offset: 0 in Drupal\csp\Csp::Drupal\csp\{closure}() (line 507 of /modules/contrib/csp/src/Csp.php)

Steps to reproduce

Just activated the module and enforce some policies.

Proposed resolution

Check line 507

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork csp-3223558

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Nchase created an issue. See original summary.

gapple’s picture

Status: Active » Postponed (maintainer needs more info)

Can you provide your config (with your own hostnames redacted if necessary)?

$ drush cget --format=yaml csp.settings

It looks like an empty string is being added as a source, which shouldn't occur just through modifying the configuration form 🤔.

gapple’s picture

The Csp class also prevents directly adding an empty string as a source (e.g. Csp::appendDirective('script-src-attr', ''); won't modify the internal state), but it is possible for some code to circumvent that check in some cases (e.g. Csp::appendDirective('script-src-attr', ' '); or Csp::appendDirective('script-src-attr', ['']);)

nchase’s picture

Exported the csp.settings. See below:

langcode: de
report-only:
  enable: true
  directives:
    script-src:
      sources:
        - 'https://s3.amazonaws.com/downloads.mailchimp.com/js/mc-validate.js'
      flags:
        - unsafe-eval
      base: self
    script-src-attr:
      base: self
    script-src-elem:
      sources:
        - 'https://s3.amazonaws.com/downloads.mailchimp.com/js/mc-validate.js'
      base: self
    style-src:
      base: self
    style-src-attr:
      base: self
    style-src-elem:
      base: self
    frame-ancestors:
      base: self
  reporting:
    plugin: none
enforce:
  enable: true
  directives:
    font-src:
      base: self
    img-src:
      sources:
        - 'https://unpkg.com'
        - 'https://a.tile.openstreetmap.org'
        - 'https://b.tile.openstreetmap.org'
        - 'https://c.tile.openstreetmap.org'
      base: self
    object-src:
      base: self
    script-src:
      sources:
        - 'https://s3.amazonaws.com/downloads.mailchimp.com/js/mc-validate.js'
      flags:
        - unsafe-eval
      base: self
    script-src-elem:
      sources:
        - 'https://s3.amazonaws.com/downloads.mailchimp.com/js/mc-validate.js'
      base: self
    frame-ancestors:
      base: self
  reporting:
    plugin: sitelog
gapple’s picture

I wasn't able to reproduce any warnings from your configuration; only expected keywords and URLs are in the array of sources passed to Csp::reduceAttrSourceList().

My only other suspect would be a subscriber to the CspEvents::POLICY_ALTER event is adding an empty string as source in a manner that I mentioned in #3

gapple’s picture

Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)

Please reopen this issue if you can provide a minimal test case to reproduce the error.
Without further info, my suspicion is that another module is not properly validating the values it is passing to CSP's API.

If there's reports of this error happening for others, I'll look into adding mitigations within this module.

seanb’s picture

Status: Closed (cannot reproduce) » Needs review
StatusFileSize
new495 bytes

I was having the same issue. It seems the "Auto sources" sometimes gets an empty item in the list. I think adding an array filter to prevent that might be a good way to protect from this.

  • gapple committed 4a899e2 on 8.x-1.x
    Issue #3223558: Handle libraries with empty file definitions better
    
gapple’s picture

I tried to dig a little further into where an empty value could be coming from via LibraryDiscovery / LibraryDiscoveryParser - I'm not sure how it could actually happen from a valid library definition, but I've added a check (and a corresponding test) for one possibility just in case.

Adding a final array_filter is a reasonable option if a more specific cause can't be addressed.

askibinski’s picture

Status: Needs review » Fixed

@gapple
Changed the status to fixed since it is in release 1.16.

askibinski’s picture

Status: Fixed » Needs review

Changed it back, looking more closely, SeanB's patch is not committed but an additional check was added like you described in #9.

BlackLotus9’s picture

I am seeing this same error appear at the top of the Extend pages in the UI.
The issue goes away when I uncheck every box in the csp module.

Warning: Uninitialized string offset 0 in Drupal\csp\Csp::Drupal\csp\{closure}() (line 515 of modules/contrib/csp/src/Csp.php).
Drupal\csp\Csp::Drupal\csp\{closure}('')
array_filter(Array, Object) (Line: 520)
Drupal\csp\Csp::reduceAttrSourceList(Array) (Line: 430)
Drupal\csp\Csp->getHeaderValue() (Line: 181)
Drupal\csp\EventSubscriber\ResponseCspSubscriber->onKernelResponse(Object, 'kernel.response', Object)
call_user_func(Array, Object, 'kernel.response', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.response') (Line: 191)
Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object, Object, 1) (Line: 179)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Details

 Drupal version   : 9.3.8                                                        
 Site URI         : https://xxx-xxx-xxx-xxx-xxx.web.ahdev.cloud 
 DB driver        : mysql                                                        
 DB hostname      : localhost                                                    
 DB port          : 3306                                                         
 DB username      : drupal                                                       
 DB name          : drupal                                                       
 Database         : Connected                                                    
 Drupal bootstrap : Successful                                                   
 Default theme    : cohesion_theme                                               
 Admin theme      : acquia_claro                                                 
 PHP binary       : /usr/local/php8.0/bin/php                                    
 PHP config       : /usr/local/php8.0/etc/cli/php.ini                            
 PHP OS           : Linux                                                        
 Drush script     : /home/ide/project/vendor/bin/drush                           
 Drush version    : 10.6.2                                                       
 Drush temp       : /tmp                                                         
 Drush configs    : /home/ide/.drush/drush.yml                                   
                    /home/ide/project/vendor/drush/drush/drush.yml               
 Install profile  : xxx                                                    
 Drupal root      : /home/ide/project/docroot                                    
 Site path        : sites/default                                                
 Files, Public    : sites/default/files                                          
 Files, Private   : /mnt/files/xxx.ide/sites/default/files-private       
 Files, Temp      : /tmp<code>

csp.settings.yml

_core:
  default_config_hash: xxx
report-only:
  enable: false
  reporting:
    plugin: none
enforce:
  enable: true
  directives:
    object-src:
      base: none
    script-src:
      base: self
    base-uri:
      base: none
  reporting:
    plugin: none
hitchshock’s picture

Status: Needs review » Reviewed & tested by the community

I have the same issue and the patch works for me. RTBC

truls1502’s picture

+1 RTBC to patch 7

gapple’s picture

Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

I would really like some detailed steps to reproduce (or ideally a unit test), since

  1. I cannot see how any empty values would be present in the array in LibraryPolicyBuilder::getSources() when they're filtered out in the protected method that it uses to populate those values, so adding array_filter seems like it would have no effect.
  2. The error would only happen when either a script-src-attr or style-src-attr directive is added to a policy.
    (The module does conditionally add them on behalf of core in some cases (Drupal.Ajax, CKEditor, quickedit), but the error would require that one of the fallback directives includes an invalid empty value in its sources)

Since the module caches the data it parses from library info, the issue needs to be reproducible after a cache clear on release 1.16 or greater.

martijn de wit’s picture

We are having the same error using the CPS module. Patch #7 doesn't seems to work. It seems patch applied cleanly.

Strange thing is that this error has been occurring for a few weeks now, and we are using the CSP module much longer in this project.

seems same warning as #12

Warning: Uninitialized string offset 0 in Drupal\csp\Csp::Drupal\csp\{closure}() (line 525 of modules/contrib/csp/src/Csp.php).
Drupal\csp\Csp::Drupal\csp\{closure}('')
array_filter(Array, Object) (Line: 530)
Drupal\csp\Csp::reduceAttrSourceList(Array) (Line: 433)
Drupal\csp\Csp->getHeaderValue() (Line: 188)
Drupal\csp\EventSubscriber\ResponseCspSubscriber->onKernelResponse(Object, 'kernel.response', Object)
call_user_func(Array, Object, 'kernel.response', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.response') (Line: 202)
Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object, Object, 1) (Line: 190)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 38)
Drupal\shield\ShieldMiddleware->bypass(Object, 1, 1) (Line: 137)
Drupal\shield\ShieldMiddleware->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 709)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

offside:
when I open csp.php, my VSC says Expected type 'array'. Found 'string'.intelephense(1006) row 408

gapple’s picture

StatusFileSize
new1.65 KB

I'm not currently inclined to commit this patch since it may mask an underlying issue, but it should adequately prevent the notice from happening for anyone who is incapable of finding the source of the empty string being added to their directives.

(as somewhat noted in the test included in the patch, this solution may prevent -attr directives from being removed to shorten the header value when the output value matches a fallback directive)

joshahubbers’s picture

In our code we did something like this:
'style-src' => 'https://somedomain.com ' . $this->getInlineStyleSha(),

But when there was no return value from getInlineStyleSha(), the added directive is
'style-src' => 'https://cloudstatic.obi4wan.com https://fonts.googleapis.com '
It ends with a space. Apparently the space is converted to an empty array item in the module.

So in all I agree that the fix masks the problem, because the extra space remains in the CSP header, and should be removed earlier in the process.

joshahubbers’s picture

StatusFileSize
new370 bytes

Please consider this solution: apply array_filter to strip the empty elements from the array in appendDirective...
Or apply the patch in #7. I don't really see why you should not add the extra prevention of empty elements. The more prevention of errors (also human errors) the better isn't it?

codebymikey’s picture

This error is triggered for my environment when the extlink module is installed, and CSP attempts to generate sources from the installed modules, however because extlink has an "external" library that's still relative to the Drupal installation i.e. /extlink/settings.js, rather than relative to the defining module.

The self::getHostFromUri() call returns an empty string, which is what triggers the error further down the process.

And as far as I can tell from reviewing core's LibraryDiscoverParser code, the library assets starting with a / may be flagged as external and still work as expected, but removing the external flag in the upstream module is ultimately the right approach to address this.

So options are:

  1. We update the code so that it automatically skips "external" libraries with no valid hosts
  2. Automatically switch empty external libraries to a "self" policy (not sure if necessary)
  3. Mark as postponed/wont-fix and leave for the relevant upstream modules to address

I would personally opt for skipping empty hosts since those contrib modules aren't necessarily opting into CSP, and since we are the ones parsing their library definitions, we should skip the ones that don't match what we want to work with.

However, thoughts are more than welcome!

P.S. Also created a draft PR so I can apply a patch locally with, but still needs tests (unfortunately don't think I'll have time to implement this).

gapple’s picture

Status: Postponed (maintainer needs more info) » Needs work

@codebymikey thanks for the investigation!

It looks like extlink should have the external type removed, or it won't work as expected if the Drupal docroot is in a folder - it currently hits the prior code block (L213-215) and skips over everything handling various internal cases (root relative, stream wrapper, extension relative, actually external but not specified as such...)

But this is easy enough to also address in CSP, so that incorrect configuration in other modules doesn't cause errors.

gapple’s picture

Priority: Normal » Minor
Status: Needs work » Postponed (maintainer needs more info)

Looks like extlink added external to work around some issues with other modules expecting a file to exist for a dynamic script that's provided by a route: #3217441: settings.js missing. I'm not sure it's a common enough use case to warrant requiring checks for file presence or adding a new attribute to library files so that extlink can revert that change 😒.

I created new child issues and committed fixes related to:
- extlink: #3377949: PHP warning if library asset provided by route is tagged as external
- appending using strings with extra whitespace: #3377953: PHP warning if adding multiple sources in string with extra spaces

I'll set this issue back to "needs more info" for anyone to report any additional causes, or (hopefully) if the current fixes have resolved the issue for them.

gapple’s picture

Status: Postponed (maintainer needs more info) » Fixed

Since it's been a few months without further reports, I'm going to close this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.