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

D10-9: Safer PHP array destructuring. #18

Merged
merged 2 commits into from
Oct 5, 2023
Merged

D10-9: Safer PHP array destructuring. #18

merged 2 commits into from
Oct 5, 2023

Conversation

adam-vessey
Copy link
Contributor

Exposed with the bump to PHP 8, started throwing warnings due to attempting to use things naively.

https://www.php.net/manual/en/language.types.array.php :

Attempting to access an array key which has not been defined is the same as accessing any other undefined variable: an E_WARNING-level error message (E_NOTICE-level prior to PHP 8.0.0) will be issued, and the result will be null.

Started popping from our migration code, as we would:

  • have a uri such as foxml://object/some:pid
  • check that it is not writable
  • naively chop to foxml://object, to attempt to assert that the parent is also not writable

Thinking: ObjectLowLevel adapters should be expected to avoid providing files which would be deletable. The akubra_adapter implementation does so... adding in some filters so the "archival" adapter should also.

Exposed with the bump to PHP 8, started throwing warnings due to
attempting to use things naively.
@adam-vessey adam-vessey added the minor Added functionality that is backwards compatible. label Sep 28, 2023
@adam-vessey adam-vessey marked this pull request as ready for review September 28, 2023 14:51
@nchiasson-dgi nchiasson-dgi merged commit 34ff16d into main Oct 5, 2023
@nchiasson-dgi nchiasson-dgi deleted the fix/d10-9 branch October 5, 2023 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Added functionality that is backwards compatible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants