cloudinary / cloudinary_php

PHP extension for Cloudinary
https://cloudinary.com/documentation/php_integration
MIT License
389 stars 150 forks source link

Bad signature when public id contains commas #361

Closed jjdiaz closed 1 year ago

jjdiaz commented 2 years ago

When the public_id contains commas the methods uploadApi()->updateMetadata and uploadApi()->upload generates an invalid signature error. However the adminApi->update method runs ok.

cloudinaraz commented 2 years ago

Hey @jjdiaz, the Admin API and the Upload API authentications are performed differently. While upload API indeed consists of a signature parameter, the Admin API uses Basic Authentication so the SDK doesn't generate the signature similar to the Upload API methods.

In general, special characters in the public_id should be encoded when making API requests, so can you please try and encode commas will be replaced with %2C and let me know if that solves the issue for you?

jjdiaz commented 2 years ago

Hi, Clodinaraz. I had already tried using urlencode without success. Please check this code snippet:

        try{
            $publicId= "Advertisements/manolito,comas";
            $encodedPublicId = urlencode($publicId);

            $result = $this->api->uploadApi()->updateMetadata(['act_test' => 'any value'], [$publicId]);
            dump($result);

        }catch (\Exception $e){
            dump($e);
        }

The result is:

Invalid Signature 43fd0c90297c7b90718923e793c7deea447e928f. String to sign - 'metadata=act_test=34343422234&public_ids=Advertisements/manolito,comas&timestamp=1655739683'.

Replacing

$result = $this->api->uploadApi()->updateMetadata(['act_test' => 'any value'], [$publicId]); by

$result = $this->api->uploadApi()->updateMetadata(['act_test' => 'any value'], [$encodedPublicId ]); No errors found but no asset affected. The dump($result) shows as follows:

] storage: array:1 [ "public_ids" => [] ]

public_ids is empty so no assets affected and no changes are made on the Cloudinary side.

Workarround:

I've rewriten the method signParameters on the class Cloudinary\Api\ApiUtils.php as follows:

public static function signParameters($parameters, $secret, $signatureAlgorithm = Utils::ALGO_SHA1)
    {
        $parameters = array_map('self::serializeSimpleApiParam', $parameters);

        ksort($parameters);

        $signatureContent = self::serializeQueryParams($parameters);

        /** jjd-Bugfix: This replacement prevents invalid signature errors when there are commas inside public_id */

        $signatureContent = str_replace("\,",",", $signatureContent);

        return Utils::sign($signatureContent, $secret, false, $signatureAlgorithm);
    }

I didn't get any errors and the asset was updated in Cloudinary BUT I think there are side effects that I'm evaluating now.

Best Regards,

momoip commented 2 years ago

Hi @jjdiaz, The call for updateMetadata looks right, but since you get no error but not find any "public_ids". Please open a support ticket and provide me the cloud name and make the call and I can investigate from our log and let you know.

jjdiaz commented 2 years ago

Hello @momoip I just opened ticket #190154

aksjoshi1 commented 2 years ago

Hi @jjdiaz Closing this issue here. We will continue on the support ticket