get(#) retrieves parameters by index rather than by key

[Jayshua]

Description

When submitting a form using numeric field names like <input name="1"><input name="0"> the browser submits the form data in url encoded format like 1=first+value&0=second+value. The get helper, when called like get(1) or with a string like get('1'), will retrieve the value at index 1 in the encoded string ('second+value' in this example) rather than with key 1 ('first+value' in this example).

Expected behavior I expected get to retrieve by key rather than by index, matching PHP's behavior when using $_POST.

My Diagnosis \Kirby\Http\Request::data() calls array_merge to combine the body and query param data into a single data array. array_merge is documented to renumber numeric keys. I believe data should call array_replace instead, which combines the arrays without renumbering them.

To reproduce

1. Replace index.php with:

   <?php
   require 'kirby/bootstrap.php';
   
   new Kirby();
   var_dump(get(1));
   var_dump(get('1'));
   var_dump($_POST[1]);
   var_dump($_POST['1']);

2. Submit a POST request in the standard application/x-www-form-urlencoded format. Here are some examples: 1=first+value&0=second+value 0=second+value&1=first+value

Your setup

Kirby Version: 3.9.2

[lukasbestle]

Thanks, looks like we hadn't expected/thought of numeric query params when implementing the logic.

[afbora]

@lukasbestle I've looked into issue. I've found the source of issue. This issue is coming from following line:

https://github.com/getkirby/kirby/blob/282b1e25faa0bc24c903237a7484dc45fa475c36/src/Http/Request.php#L196-L199

As as solution, I think we need to use array_replace instead array_merge. I've tested and seems works great. But I'm not sure if this change will break anything. What do you think?

array_merge documentation: values in the input arrays with numeric keys will be renumbered with incrementing keys starting from zero in the result array.

[lukasbestle]

The only difference between array_merge() and array_replace() that I'm aware of is the handling of numeric keys. Since we want PHP to treat keys as strings, not as numbers, array_replace() is indeed the correct choice here IMO.