Skip to content

Fix Collivery auth handling: guard missing data, add cURL timeouts, and treat invalid JSON as API error to prevent warnings and long hangs. #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RichardGeorgeDavis wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix Collivery auth handling: guard missing data, add cURL timeouts, and treat invalid JSON as API error to prevent warnings and long hangs. #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd5c3d3
Fix Collivery auth handling: guard missing data, add cURL timeouts, a…
RichardGeorgeDavis Feb 10, 2026
19c7bfc
Harden Collivery caching/auth and fix cURL GET leak
RichardGeorgeDavis Feb 10, 2026
316fb75
Include rotated logs in zip and clear operations
RichardGeorgeDavis Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 94 additions & 16 deletions MdsSupportingClasses/Collivery.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class Collivery
protected $config;
protected $errors = [];
protected $check_cache = true;
protected $cache;
protected $last_http_code;
protected $last_request_url;

protected $default_address_id;
protected $client_id;
Expand Down Expand Up @@ -81,6 +84,14 @@ protected function authenticate()
if (
$this->check_cache &&
$this->cache->has('collivery.auth') &&
is_array($authCache) &&
isset(
$authCache['email_address'],
$authCache['client']['primary_address']['id'],
$authCache['client']['id'],
$authCache['id'],
$authCache['api_token']
) &&
$authCache['email_address'] == $this->config->user_email
) {
$this->default_address_id = $authCache['client']['primary_address']['id'];
Expand Down Expand Up @@ -112,6 +123,7 @@ private function consumeAPI($url, $data, $type, $isAuthenticating = false) {
: rtrim(self::BASE_URL, '/');
$endpoint = ltrim($url, '/');
$url = $base . '/' . $endpoint;
$this->last_request_url = $url;

if (!$isAuthenticating) {
if ($this->token) {
Expand All @@ -127,13 +139,13 @@ private function consumeAPI($url, $data, $type, $isAuthenticating = false) {
}
}

$client = curl_init($url);

if ($type == 'POST') {
$client = curl_init($url);
curl_setopt($client, CURLOPT_POST, 1);
$data = json_encode($data);
curl_setopt($client, CURLOPT_POSTFIELDS, $data);
} else if ($type == 'PUT') {
$client = curl_init($url);
curl_setopt($client, CURLOPT_CUSTOMREQUEST, 'PUT');
$data = json_encode($data);
curl_setopt($client, CURLOPT_POSTFIELDS, $data);
Expand All @@ -156,39 +168,42 @@ private function consumeAPI($url, $data, $type, $isAuthenticating = false) {

curl_setopt($client, CURLOPT_HTTPHEADER, $headerArray);

// Prevent long hangs when the API is slow/unreachable.
$connectTimeout = isset($this->config->connect_timeout) ? (int) $this->config->connect_timeout : 10;
$timeout = isset($this->config->timeout) ? (int) $this->config->timeout : 20;
curl_setopt($client, CURLOPT_CONNECTTIMEOUT, max(1, $connectTimeout));
curl_setopt($client, CURLOPT_TIMEOUT, max(1, $timeout));

$result = curl_exec($client);

$this->last_http_code = curl_getinfo($client, CURLINFO_HTTP_CODE);

if (curl_errno($client)) {
$errno = curl_errno($client);
$errmsg = curl_error($client);
curl_close($client);

throw new CurlConnectionException('Error executing request', 'ConsumeAPI()', [
throw new CurlConnectionException('Error executing request: '.$errmsg, 'ConsumeAPI()', [
'Code' => $errno,
'Message' => $errmsg,
'URL' => $url,
'Result' => $result
]);
}

if (isset($result['error'])) {
$error = $result['error'];
throw new CurlConnectionException('Error executing request', 'ConsumeAPI()', [
'Code' => $error['http_code'],
'Message' => $error['message'],
'URL' => $url,
'Result' => $result
]);
}

curl_close($client);

// If $result is already an array.
if (is_array($result)) {
return $result;
}

return json_decode($result, true);
$decoded = json_decode($result, true);
if (!is_array($decoded)) {
return ['error' => ['http_code' => 500, 'message' => 'Invalid response from Collivery API']];
}

return $decoded;
}

/**
Expand Down Expand Up @@ -217,6 +232,18 @@ public function makeAuthenticationRequest($settings = null)
"password" => $user_password
], 'POST', true);

if (!is_array($authenticate)) {
$this->setError('result_unexpected', 'No result returned.');
$this->logAuthFailure($authenticate, $user_email);
return [];
}

if (!isset($authenticate['data'])) {
$this->checkError($authenticate);
$this->logAuthFailure($authenticate, $user_email);
return [];
}

$authenticate = $authenticate['data'];

if (is_array($authenticate) && isset($authenticate['api_token'])) {
Expand All @@ -241,11 +268,57 @@ public function makeAuthenticationRequest($settings = null)
}
} catch (CurlConnectionException $e) {
$this->catchException($e);
$this->logAuthFailure(['exception' => $e->getMessage()], $user_email);
}

return [];
}

/**
* Log additional diagnostics for authentication failures.
*
* @param mixed $response
* @param string $user_email
*/
private function logAuthFailure($response, $user_email)
{
$logger = new MdsLogger();
$headers = [
'X-App-Name' => $this->config->app_name,
'X-App-Version' => $this->config->app_version,
'X-App-Host' => $this->config->app_host,
'X-App-Lang' => 'PHP '.phpversion(),
'X-App-Url' => $this->config->app_url,
];

if (!is_array($response)) {
$response = ['raw' => $response];
}

$logger->warning(
'Collivery::makeAuthenticationRequest',
'Authentication request failed',
[
'app_name' => $this->config->app_name,
'app_version' => $this->config->app_version,
'app_host' => $this->config->app_host,
'app_url' => $this->config->app_url,
'base_url' => $this->config->base_url,
],
[
'request' => [
'url' => $this->last_request_url,
'email' => $user_email,
'headers' => $headers,
'password' => '[redacted]',
],
'response' => $response,
'http_code' => $this->last_http_code,
'errors' => $this->errors,
]
);
}

/**
* Returns true or false if authenticated by making a new authentication request.
*
Expand Down Expand Up @@ -1111,6 +1184,9 @@ protected function catchException($e)
private function checkError($data) {
if (isset($data['error'])) {
$this->setError($data['error']['http_code'], $data['error']['message']);
} elseif (isset($data['message'])) {
$code = $this->last_http_code ? $this->last_http_code : 'result_unexpected';
$this->setError($code, $data['message']);
} else {
$this->setError('result_unexpected', 'No result returned.');
}
Expand Down Expand Up @@ -1243,10 +1319,12 @@ public function make_key_value_array($data, $key, $value, $isContact = false) {
/**
* Returns the Collivery User Id for the credentials used by the store owner.
*
* @return Integer - The User Id;
* @return int|null - The User Id;
*/
public function getColliveryUserId() {
return $this->authenticate()['id'];
$auth = $this->authenticate();

return (is_array($auth) && isset($auth['id'])) ? $auth['id'] : null;
}

public function filterServices(array $services)
Expand Down
33 changes: 31 additions & 2 deletions MdsSupportingClasses/MdsCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,40 @@ class MdsCache
*/
public function __construct($cache_dir = 'cache/mds_collivery/')
{
if ($cache_dir === null) {
if ($cache_dir === null || $cache_dir === '') {
$cache_dir = 'cache/mds_collivery/';
}

$this->cache_dir = $cache_dir;
// Resolve relative paths to a writable base when possible.
if (!$this->is_absolute_path($cache_dir)) {
if (defined('WP_CONTENT_DIR') && WP_CONTENT_DIR) {
$cache_dir = rtrim(WP_CONTENT_DIR, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . ltrim($cache_dir, '/\\');
} else {
$cache_dir = rtrim(sys_get_temp_dir(), DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . 'mds_collivery/';
}
}

$this->cache_dir = rtrim($cache_dir, '/\\') . DIRECTORY_SEPARATOR;
}

/**
* Determine if a path is absolute.
*
* @param string $path
*
* @return bool
*/
private function is_absolute_path($path)
{
if ($path === '') {
return false;
}

if ($path[0] === '/' || $path[0] === '\\') {
return true;
}

return (bool) preg_match('/^[A-Za-z]:[\/\\\\]/', $path);
}

/**
Expand Down
14 changes: 13 additions & 1 deletion MdsSupportingClasses/MdsFields.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ public static function getFields()
'placeholder' => admin_url().'admin.php?page=mds_download_log_files',
'default' => null,
],
'debug_logging' => [
'title' => __('Debug logging'),
'type' => 'checkbox',
'description' => __('When enabled, logs include request/response details and can grow quickly.'),
'default' => 'no',
],
'enabled' => [
'title' => __('Enabled?'),
'type' => 'checkbox',
Expand Down Expand Up @@ -322,8 +328,14 @@ public static function getResources(MdsColliveryService $service)
foreach (['towns', 'location_types', 'services'] as $resource) {
$result = $collivery->{'get'.str_replace('_', '', ucwords($resource))}();
if (!is_array($result)) {
$errorMessage = 'Unable to retrieve fields from the API';
$errors = $collivery->getErrors();
if (!empty($errors)) {
$errorMessage .= ': '.implode('; ', array_values($errors));
}

throw new InvalidResourceDataException(
'Unable to retrieve fields from the API',
$errorMessage,
$service->loggerSettingsArray()
);
}
Expand Down
Loading