When loading local pages that call stage file proxy a warning is thrown as NULL is being passed to a trim function.
Deprecated function: trim(): Passing null to parameter #1 ($string) of type string is deprecated in Drupal\stage_file_proxy\EventSubscriber\ProxySubscriber->checkFileOrigin() (line 135 of modules/contrib/stage_file_proxy/src/EventSubscriber/ProxySubscriber.php).
Drupal\stage_file_proxy\EventSubscriber\ProxySubscriber->checkFileOrigin(Object, 'kernel.request', Object)
call_user_func(Array, Object, 'kernel.request', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.request') (Line: 134)
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: 49)
Asm89\Stack\Cors->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: 49)
To prevent warnings check for NULL before passing to trim function.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | interdiff-3293275-2-11.txt | 578 bytes | markdorison |
| #11 | stage_file_proxy-php81-trim-warnings-3293275-11.patch | 769 bytes | markdorison |
| #2 | stage_file_proxy-php81-trim-warnings-3293275-2.patch | 730 bytes | 3li |
Show commandsStart 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:
changes
Comments
Comment #2
3liComment #3
3liComment #4
gregglesWhy is the origin dir configured to be null?
Comment #5
albeorte commentedComment #6
markdorisonComment #7
markdorisonWhen the module is first enabled won't that value be empty and return NULL?
Comment #8
gregglesI guess so. If it is, should the module silently ignore the problem or should it be noisy that the configuration needs to be set?
Comment #9
markdorisonIn my opinion, I think it is an expected state that we should account for, rather than a problem.
Comment #10
gregglesYeah, I think we agree it's an expected state we should account for.
As I understand the proposal this change would account for it by silently using the value which will then cause the remote requests to fail.
It seems another way to account for the expected state would be to display an error or log an error that encourages the admin to configure the module.
I don't feel super passionately about this perspective, though, and I'd be fine with this getting committed (though the test failures need to be reviewed).
Comment #11
markdorisonI see what you are driving at. It's that the change in patch #2 is going to change the behavior of the subsequent check on
$remote_file_dir.I am not against this approach but is there a way to complete this issue without changing the functionality and then address a change like that in a separate issue? Would an additional change to our check on
$remote_file_dir(see included patch) resolve the warning while also preserving the existing functionality?Comment #12
smustgrave commentedJust to chim in we did last night change the 8.x-1.x branch to be D9 only and started a new 2.0.x branch that will be D10 only. In case that plays into this conversation some.
Comment #13
smustgrave commentedComment #16
smustgrave commentedAdded to the 2.0.x branch!