Avoid error when $options is NULL in buildUrl()

Restoring issue summary, tags and related issue which was removed in #68.

Re-roll patch needed for 11.x.

Re-roll patch created for 11.x.

I added the Needs issue summary update tag because it would help to understand remaining work and the proposed resolution specifically the test coverage expansion in the issue summary rather in the comment chain.

I am working with @ompiepy at MidCamp 2024 and we're going to look at this issue to try to move it forward. Possibly transitioning to a merge request after updating the issue summary.

I am looking into the issues to get the summary update and then start working for the merge request.

samit.310@gmail.com → made their first commit to this issue’s fork.

Did not review

Issue was tagged for issue summary update which appears to still be needed.

To handle the error when $options is NULL in buildUrl(), you can add a simple check to ensure that $options is always an array.
Something like this

$options = is_array($options) ? $options : [];

The patch we suggested seven years ago (!), simply casting to array, was already capable of accomplishing this. :-)

I have long moved on since and no longer bitten by this problem but this one is really ridiculous, I have to say. A very straightforward modification that obviously doesn't even require testing, it cannot cause any problems or regresssion by definition, and nobody ever bothered to accept it (or even argue against it). Must be one of the record setters in Drupalland. :-)

In response to #3 and #76:

The options of an URL object are documented as optional, which means NULL is a valid value.

So it does not really matter where the underlying problem is coming from. The problem is that the code blindly assumes that $options is a mandatory array, which does not comply with the spec. So any code in core that assumes that options is array, should be rewritten to be null-safe.

I got into this ticket due LinkGenerator, which has exactly the same incorrect not-null assumptions (line 93 and line 154)

So IMHO the underlying problem is not that somwhere the options aren't set, the underlying problem is that this code is not following the specification. I've added this to the issue summary.

patch for 11.2 without tests

> The options of an URL object are documented as optional, which means NULL is a valid value.

It doesn't mean NULL is valid, because the default $options is an empty array. Eventually we will be adding types and this argument will be typed as an array.

This means it is the caller's responsibility to ensure an array is passed in. In the LinkFormatter case, the underlying options property in LinkItem is a Map, which is stored either as an array of values or NULL. Therefore I think the fix should be in LinkFormatter::buildUrl():

    $options = $item->options ?? [];

I did a deep dive on this issue and discovered problems with the assumptions and the work that has been done so far in this issue. I'm fortunate to be debugging this with later versions of PHP that provide better error messages. The issue summary has been properly updated with detailed descriptions of the problems at hand.

The first important thing to understand is that LinkFormatter is not receiving NULL as the options value. It's actually FALSE. As far as I can tell it has always been FALSE. In fact, none of the above patches that explicitly treated the value as being NULL, i.e. corrected it with the NULL coalescing operator, were reported as working. The only "working" patches are ones that just happen to correct the FALSE case.

The second important thing to note is that this really isn't an issue with the formatter. Any attempt to correct the problem in it will only cause the unserialization warnings to become more apparent. Then eventually unserialize() will start throwing errors instead of warnings and we'll be back to having WSoDs again. The inherent problem is lower-level than that. We allow a field column to be NULL, but the DB layer always tries to unserialize the value. I don't know that this is even the DB layer's fault. It may be the field's for not requiring it. I have to solicit opinions about how it should be fixed.

Postponing on 📌 Unserialize(): Passing null to parameter #1 ($data) of type string is deprecated in Drupal\Core\Entity\Sql\SqlContentEntityStorage::loadFromSharedTables() Needs work which should correct the unserialization problem. If the proposed fix for that one is implemented, then the options value will actually be NULL and we can proceed from there.