2
\$\begingroup\$

I have a basic JSON API written in PHP. I want to validate incoming JSON data.

I know there are PHP JSON validators out there, here is a quick one I just rolled up.

<?php

$json_payload = json_decode('{
  "data": {
    "a_string": "short",
    "a_number": 7
  }
}');

validateJSONPayload(
  '{
    "data": {
      "a_string": "type=string,min_length=7,max_length=10",
      "a_number": "type=integer,min=8,max=50"
    }
  }',
  $json_payload
);

function validateJSONPayload($json_schema, $json_payload){

  $json_schema = json_decode($json_schema);

  function reCursiveCheck($schema_value, $schema_type, $payload_value, $payload_type){
    if($schema_type === "object" || $schema_type === "array"){
      # KEEP LOOPING
      foreach ($schema_value as $key => $key_val) {
        if(!property_exists($payload_value, $key)) echo "validation failed because the data does not match the schema<br />";
        reCursiveCheck($key_val, gettype($key_val), $payload_value->{$key}, gettype($payload_value->{$key}));
      }
    } else {
      # VALUE IS A STRING, NUMBER, BOOL, NULL
      $validation_param_strings = explode(",", $schema_value);

      # STORE THE PARAMS TO CALL THE VALIDATION FUNCTION
      $validation_params = array();

      foreach($validation_param_strings as $validation_param) {
        $params = explode("=", $validation_param);
        $validation_params[$params[0]] = $params[1];
      }
      // print_r($validation_params);
      validateProperty($payload_value, $payload_type, $validation_params);
    }
  }

  function validateProperty($payload_value, $payload_type, $validation_rule){
    if($payload_type !== $validation_rule["type"]) {
      echo "types do not match<br />";
    }
    # STRING VALIDATION
    if($validation_rule["type"] === "string"){
      if(strlen($payload_value) > $validation_rule["max_length"]) echo "string too long<br />";
      if(strlen($payload_value) < $validation_rule["min_length"]) echo "string too short<br />";
    }
    # NUMBER VALIDATION
    if($validation_rule["type"] === "integer"){
      if($payload_value > (int)$validation_rule["max"]) echo "number too large<br />";
      if($payload_value < (int)$validation_rule["min"]) echo "number too small<br />";
    }
    # NULL VALIDATION
    if($validation_rule["type"] === "null"){
      echo "found a null<br />";
    }
  }

  foreach($json_schema as $key => $value) {
    if(!property_exists($json_payload, $key)) echo "validation failed because the data does not match the schema<br />";
    reCursiveCheck($value, gettype($value), $json_payload->{$key}, gettype($json_payload->{$key}));
  }
}

?>
\$\endgroup\$
1
\$\begingroup\$

Your solution is very "functional programming" in approach, with nested function declarations and such. This in of itself is not "wrong", it is just a little bit unusual in the PHP world.

I would consider moving this into an object-oriented paradigm which is more common in PHP. I would think in this case perhaps only a class with static functions might be needed, so you could make calls against the validator like:

$result = JsonSchemaValidator::validate($jsonSchema, $jsonPayload);

If you find you have one schema that is used multiple times for validation, then perhaps it makes sense to have concrete validator objects like this:

$validator = new JsonSchemaValidator($jsonSchema);
$result1 = $validator->validate($jsonPayload1);
$result2 = $validator->validate($jsonPatload2);

This would save you the overhead of having to re-validate the schema with each validation request.

I am not sure which is more appropriate to your application, but wanted to present both options for putting this logic into a class.


Some odd naming conventions are in play:

  • Why name a variable $json_payload when it does not contain JSON at all, but rather an object built from JSON? Perhaps $payload is better here.
  • Why are you mixing camelCase and snake_case in your code? I know PHP is not very consistent here, but that doesn't mean you shouldn't be - at least within the confines of a single library, class, set of functions, etc.
  • Why uppercase first c in reCursiveCheck()?

I think your main validateJSONPayload() function name and signature is odd. As noted above, the "payload" is not JSON at all. Why pass one parameter as a JSON string and the other as an actual data structure? This seems to be an incongruous approach. I would probably just pass JSON for both parameters so caller doesn't need to decode one of the parameters beforehand.


Why define validation rules in a string like "type=string,min_length=7,max_length=10"? You are introducing complexity in your code to have to perform string manipulation to get at these values when they should be defined as appropriate properties/values on the schema. Why not mirror the already well-established JSON Schema format?


Do not echo error/validation messages to standard output. A validator like this should do one thing - validate, not deliver end user messaging. Leave that up to functionality further up the call stack that is more well-positioned to understand the context for how end user messaging should be delivered. This code might throw exceptions and/or log errors when it gets put into a bad state (i.e. invalid JSON is passed) but outside of that, it's contract to the caller should be to just deliver validation results and leave it up to the calling code to determine what action to take from there.

If you are delivering error messages, you should also remove any HTML markup from it. What if this code is being used in a RESTful service where response is going to be JSON format and not HMTL? Now you have to go strip out the HTML mark-up that has been added.

The takeaway here is to separate your display concerns from your business logic.


This code is incredibly fragile and strictly "happy path" in nature, as you are doing no validation of the passed dependencies and you just assume that all of the steps in the code will work properly. Most critically, you just assume the passed schema JSON will decode properly.

What if it doesn't?

Right now, your code will just silently fail, perhaps giving the caller the perception that validation passed, since you currently do not return any validation result to the caller (you just echo out failures).

To extend on my earlier class-based example, perhaps you need to do something like:

try {
    $result = JsonSchemaValidator::validate($jsonSchema, $jsonPayload);
} catch (InvalidArgumentException $e) {
    // perhaps we had schema or payload which could not be decoded
    // and we have the code throw InvalidArgumentExpection
    // perhaps log the error
    error_log($e->getMessage());
    // then perhaps do something to recover
    $result = false;
}

or

try {
    $validator = new JsonSchemaValidator($jsonSchema);
    $result1 = $validator->validate($jsonPayload1);
    $result2 = $validator->validate($jsonPatload2);
} catch (InvalidArgumentException) {
    // do something
}

I echo the sentiment mentioned in comments to question above that I am really not quite sure why you would want to roll your own solution to this problem.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.