fix: Handle non-array JSON in validation

[woodongwong]

Description
A check has been added in the JSON validation method to ensure that the data being validated is an array. If the parsed JSON is not an array , it gets defaulted to an empty array to prevent further errors in the validation process.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

Thank you for sending PR.

But why do you need to convert invalid JSON string to an empty array?
If PR #8153 is merged, this PR will be useless.

[woodongwong]

If $this->data is not an array, there will be an error here: https://github.com/codeigniter4/CodeIgniter4/blob/1379b7a18a836fa27ea8c70ab297bf55702271ce/system/Validation/Validation.php#L207C28-L207C28

[woodongwong]

$this->data only makes sense if it is of type array.

[kenjis]

Thank you for sending this PR.

But we merged #8153, and now $request->getJSON(true) throws an exception.
And this PR does not pass the GitHub Action checks.

   1) system/Validation/Validation.php (unary_operator_spaces, not_operator_with_successor_space, no_whitespace_in_blank_line)
      ---------- begin diff ----------
--- /home/runner/work/CodeIgniter4/CodeIgniter4/system/Validation/Validation.php
+++ /home/runner/work/CodeIgniter4/CodeIgniter4/system/Validation/Validation.php
@@ -496,10 +496,10 @@
         if (strpos($request->getHeaderLine('Content-Type'), 'application/json') !== false) {
             $this->data = $request->getJSON(true);
 
-            if (!is_array($this->data)) {
+            if (! is_array($this->data)) {
                 $this->data = [];
             }
-            
+
             return $this;
         }
 

      ----------- end diff -----------


Found 1 of 877 files that can be fixed in 74.041 seconds, 46.000 MB memory used
Error: Process completed with exit code 8.

https://github.com/codeigniter4/CodeIgniter4/actions/runs/6796578870/job/18817608840?pr=8176

[kenjis]

Do you still need this PR?

[woodongwong]

var_dump(json_decode('2023'));     // int(2023)
var_dump(json_last_error_msg());   // string(8) "No error"

In this scenario, the JSON can be decoded successfully, but it has no significance for validation.

[kenjis]

What should we do when $this->data is not an array?
Is it okay just to throw the value away?

[woodongwong]

In my opinion, if we have decided to use Validation, it must be in the key-value format. If it is a special format, Validation is not applicable. Validation doesn't need to be as comprehensive as JSON schema.

[kenjis]

FYI: We do not recommend to use withRequest():
https://codeigniter4.github.io/CodeIgniter4/libraries/validation.html#withrequest

[MGatner]

I've already shared my opinion that this is an issue due to mixed concerns, but since we are currently supporting key-value request validation I think this is an acceptable "filter".

[kenjis]

if we have decided to use Validation, it must be in the key-value format. If it is a special format, Validation is not applicable.

Agreed. That is, if a scalar comes, it should be invalid format.
In terms of security principles, we should not correct invalid values.
We should stop processing.

[MGatner]

stop processing

That sounds to me like throwing an exception, rather than passing validation quietly.

[woodongwong]

I agree with MGatner's point of view, throwing exceptions is an appropriate way to handle it. Should I throw \CodeIgniter\HTTP\Exceptions\HTTPException? I found that HTTPException carries various exceptions, which is not conducive to catching (another topic).

[kenjis]

There are many problems with the current CI4 Exceptions.
Exception classes are not clearly designed.
RuntimeException and LogicException are not properly distinguished.
One exception class is used for multiple purposes.
It is a bit difficult to add appropriate exception class at will.

[woodongwong]

I organized the code and created a new PR, #8288 .