3
\$\begingroup\$

I am writing a PHP service which will listen for JSON requests (made using jQuery) and will return the JSON responses.

The whole template for such services (there will be 5) I have shortened into this template:

<?php

// set the headers which are accepted
header("Access-Control-Allow-Origin: *");
header("Access-Control-Allow-Headers: origin, content-type, accept, authorization");
//header("Access-Control-Allow-Methods: GET, POST, PUT, DELETE, OPTIONS, HEAD");
header("Access-Control-Allow-Methods: POST");
header('Content-Type: application/json');

// "HTTPS-only " validation
$isSecure = false;
if (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') {
    $isSecure = true;
}
elseif (!empty($_SERVER['HTTP_X_FORWARDED_PROTO']) && $_SERVER['HTTP_X_FORWARDED_PROTO'] == 'https' || !empty($_SERVER['HTTP_X_FORWARDED_SSL']) && $_SERVER['HTTP_X_FORWARDED_SSL'] == 'on') {
    $isSecure = true;
}

$REQUEST_PROTOCOL = $isSecure ? 'https' : 'http';

if (!$isSecure) {
    $response['status'] = 1;
    $response['message'] = 'Connection is not secure';
    http_response_code(400);
    echo json_encode($response);
    exit;
}

// !! for debug only !!
error_reporting(E_ALL);

// get the request data
$postdata = file_get_contents("php://input");

// decode
$request = json_decode($postdata, true);

$response = [];
try {
    if (true) {
        throw new Exception();  // validation not passed case
    }

    // proccess request data .. make calculations

    // all went successfull case
    $response['status'] = 0;
    $response['message'] = 'OK';
} catch (Exception $ex) {
    $response['status'] = 3;
    $response['message'] = 'Exception';
} finally {
    if ($response['status'] === 0) {
        // set the response code to 200: OK
        http_response_code(200);
    } else {
        http_response_code(400);
    }

    // output the answer
    echo json_encode($response);
}

Here is the approach I am using:

  1. set the allowed headers
  2. allow only HTTPS based calls (no HTTP)
  3. decode the input data
  4. make data validation and analysis
  5. throw a custom Exception if some validation is not passed
  6. in finally block echo the accumulated response variable

I am quite experienced when it comes down to PHP services so I am looking for an advice - if the above mentioned pattern/template can be improved in any way? Add additional validation?

The basic use cases are: new user registration/login/change password/get data specific to provided object ID, pushing some data to DB etc.

\$\endgroup\$
  • \$\begingroup\$ This doe not look like a complete code review. You look to have removed pertinent sections of the code, thus making your current code example inoperable - it will always send the Exception error message if you make it to the point in code where you being to build the response array. \$\endgroup\$ – Mike Brant Sep 26 '16 at 20:26
1
\$\begingroup\$

If you are truly trying to decouple the request handling and response building from the service definition (i.e. build a template), you might want common classes which could accept some service configuration information containing things such as:

  • CORS configuration
  • Security configuration (is secure request required)
  • Support HTTP action verbs

And perform some of the basics around building the request and response objects

Imagine a template which looks like this:

// likely stored in configuration somewhere
$request_config = (object) [
    'https_required' => true,
    'https_forward_allowed' => true
    'http_accept' => [
        'application/json' => true,
        'xml/text' => false
    ],
    // other configurations you may need to validate a request or perhaps
    // determine its routing
];

$response_config = (object) [
    'cors_headers' => [
        'Access-Control-Allow-Origin: *',
        'Access-Control-Allow-Headers: origin, content-type, accept, authorization',
        'Access-Control-Allow-Methods: POST'
    ],
    'content-type' => 'application/json'
    // any other things you may want to configure in response
];

// instantiate response object based on configuration
try {
    // perhaps throw exception if configuration is bad
    $response = new Response($response_config);
} catch (Exception $e) {
    error_log(
        'Unable to build Response object. ' .
        'Exception message: "' . $e-getMessage . '"'
    );
    http_response_code(500);
    exit;
}

// try to build a valid request object
try {
    // instantiate request object
    // this instantiation should throw Exception (with reason) if you
    // get an unexpected request.
    // this request object constructor could check request headers
    // against expected values, try to to marshall (json_decode) the POSTed data
    // etc. and fail out with exception if request falls outside expected.
    $request = new Request($request_config);
} catch (Exception $e) {
    error_log(
        'Unable to build Request object. ' .
        'Exception message: "' . $e-getMessage . '"'
    );
    $response->set_error_code(400);
    $response->set_error_message('Bad Request');
    // call method which sends response, but would also be responsible
    // for sending appropriate method (like with internal class class to 
    // (send_headers()) or similar method.
    $response->send();
    exit;
}

// now you have valid Request object you can work with and do things like:
$post_data = $request->get_post_data();

// maybe pass to the posted data to some class or otehr logic which builds
// a data structure for return.
$response->set_data(json_encode($some_data));
$response->send();
exit;

Note that the thought here is to define classes which can encapsulate the logic around validating the request against expected request headers. You make this functionality more usable and extensable in this fashion. For example, say you want to add an authentication/authorization component in your application. You could ideally just have to modify the Request class to handle the extra logic around validating authentication information.

Outside of that, a few other thoughts:

  • $REQUEST_PROTOCOL = $isSecure ? 'https' : 'http'; What is this here for as it doesn't seem to be used anywhere else.
  • You have a few lines of code that are really long. Try to keep your lines under 80 characters.
  • You seem to start building your $response array out of order. Where you define the empty array is several lines past the first time you try to set information into that array.
\$\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.