Skip to content

[Notifier][Clickatell] Invalid or missing parameter: to #57393

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.

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

Closed
jsunier opened this issue Jun 14, 2024 · 2 comments
Closed

[Notifier][Clickatell] Invalid or missing parameter: to #57393

jsunier opened this issue Jun 14, 2024 · 2 comments

Comments

@jsunier
Copy link

jsunier commented Jun 14, 2024

Symfony version(s) affected

>=6.4.x

Description

I am using the symfony/clickatell-notifier bridge and i came across an error while trying to send an SMS to any valid phone number.

{
  "error": {
      "code": "101",
      "description": "Invalid or missing parameter: to",
      "documentation": "http://www.clickatell.com/help/apidocs/error/101.htm"
  }
}

How to reproduce

  1. install the symfony/clickatell-notifier
  2. configure it in config/packages/notifier.yaml
  3. send a simple SMS to a valid phone number
  4. an error Invalid or missing parameter: to is returned by the Clickatell API
$sms = new Notification('Test message', ['sms/clickatell']);

$this->notifier->send($sms, new Recipient('', '+41780000000'));

+41780000000 is not a valid number, it's only for showing an example.

Full example to test with any number / SMS body :

Notifier configuration :

framework:
    notifier:
        # chatter_transports:
        texter_transports:
            clickatell: '%env(CLICKATELL_DSN)%'
            fakesms+email: '%env(FAKE_SMS_DSN)%'
        #    nexmo: '%env(NEXMO_DSN)%'
        channel_policy:
            # use chat/slack, chat/telegram, sms/twilio or sms/nexmo
            urgent: ['email']
            high: ['email', 'sms/clickatell']
            medium: ['email', 'sms/clickatell']
            low: ['email']

Symfony command (app:notifier:sms) for sending SMS using CLI :

<?php

namespace App\Command;

use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\Notifier\Exception\TransportExceptionInterface;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\NotifierInterface;
use Symfony\Component\Notifier\Recipient\Recipient;

#[AsCommand('app:notifier:sms', 'Send an SMS using Symfony Notifier component')]
class NotifierSmsCommand extends Command
{
	public function __construct(private readonly NotifierInterface $notifier)
	{
		parent::__construct();
	}

	protected function configure(): void
	{
		$this
			->addArgument('phone_number', InputArgument::REQUIRED, 'Phone number')
			->addArgument('subject', InputArgument::REQUIRED, 'Content of the SMS')
			->addOption('channel', 'c', InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, 'Channel to use for sending notification', ['sms/clickatell'])
		;
	}

	protected function execute(InputInterface $input, OutputInterface $output): int
	{
		$io = new SymfonyStyle($input, $output);
		$phoneNumber = $input->getArgument('phone_number');
		$subject = $input->getArgument('subject');
		$channels = $input->getOption('channel');

		$sms = new Notification($subject, $channels);

		try {
			$this->notifier->send($sms, new Recipient('', $phoneNumber));
		} catch (TransportExceptionInterface $e) {
			$io->error($e->getMessage());

			return Command::FAILURE;
		}

		$io->success('SMS sent successfully !');

		return Command::SUCCESS;
	}
}

Possible Solution

After some digging, i found the issue while reading Clickatell SMS REST API documentation.

In their example, the to parameter is an array:

{"text":"Test Message","to":["2799900001", "2799900002"]}

However a seen in the https://github.com/symfony/clickatell-notifier/blob/b180e5aeacce4dfd600bc1fcdc358ceac86472b8/ClickatellTransport.php#L64
the to parameter is sent as a string an not an array.
So the resulting request body is

{"text":"Test Message","to":"2799900001"}

which is invalid.

I can see that the to parameter was changed in this commit symfony/clickatell-notifier@1f6b83e.

Solution

Replace the line 64 in https://github.com/symfony/clickatell-notifier/blob/b180e5aeacce4dfd600bc1fcdc358ceac86472b8/ClickatellTransport.php#L64

<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\Notifier\Bridge\Clickatell;

use Symfony\Component\Notifier\Exception\TransportException;
use Symfony\Component\Notifier\Exception\UnsupportedMessageTypeException;
use Symfony\Component\Notifier\Message\MessageInterface;
use Symfony\Component\Notifier\Message\SentMessage;
use Symfony\Component\Notifier\Message\SmsMessage;
use Symfony\Component\Notifier\Transport\AbstractTransport;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;

/**
 * @author Kevin Auvinet <k.auvinet@gmail.com>
 */
final class ClickatellTransport extends AbstractTransport
{
    protected const HOST = 'api.clickatell.com';

    public function __construct(
        #[\SensitiveParameter] private string $authToken,
        private ?string $from = null,
        ?HttpClientInterface $client = null,
        ?EventDispatcherInterface $dispatcher = null,
    ) {
        parent::__construct($client, $dispatcher);
    }

    public function __toString(): string
    {
        if (null === $this->from) {
            return sprintf('clickatell://%s', $this->getEndpoint());
        }

        return sprintf('clickatell://%s%s', $this->getEndpoint(), null !== $this->from ? '?from='.$this->from : '');
    }

    public function supports(MessageInterface $message): bool
    {
        return $message instanceof SmsMessage;
    }

    protected function doSend(MessageInterface $message): SentMessage
    {
        if (!$message instanceof SmsMessage) {
            throw new UnsupportedMessageTypeException(__CLASS__, SmsMessage::class, $message);
        }

        $endpoint = sprintf('https://%s/rest/message', $this->getEndpoint());

        $options = [];
        $options['from'] = $message->getFrom() ?: $this->from;
-       $options['to'] = $message->getPhone();
+       $options['to'] = [$message->getPhone()];
        $options['text'] = $message->getSubject();

        $response = $this->client->request('POST', $endpoint, [
            'headers' => [
                'Accept' => 'application/json',
                'Authorization' => 'Bearer '.$this->authToken,
                'Content-Type' => 'application/json',
                'X-Version' => 1,
            ],
            'json' => array_filter($options),
        ]);

        try {
            $statusCode = $response->getStatusCode();
        } catch (TransportExceptionInterface $e) {
            throw new TransportException('Could not reach the remote Clickatell server.', $response, 0, $e);
        }

        if (202 === $statusCode) {
            $result = $response->toArray();
            $sentMessage = new SentMessage($message, (string) $this);
            $sentMessage->setMessageId($result['data']['message'][0]['apiMessageId']);

            return $sentMessage;
        }

        $content = $response->toArray(false);
        $errorCode = $content['error']['code'] ?? '';
        $errorInfo = $content['error']['description'] ?? '';
        $errorDocumentation = $content['error']['documentation'] ?? '';

        throw new TransportException(sprintf('Unable to send SMS with Clickatell: Error code %d with message "%s" (%s).', $errorCode, $errorInfo, $errorDocumentation), $response);
    }
}

Additional Context

Without fix

Capture d’écran 2024-06-14 105040

With fix

Capture d’écran 2024-06-14 105441

Suspected commit

symfony/clickatell-notifier@1f6b83e

@xabbuh
Copy link
Member

xabbuh commented Jun 14, 2024

Can you confirm that #57395 fixes this?

@jsunier
Copy link
Author

jsunier commented Jun 14, 2024

Can you confirm that #57395 fixes this?

I confirm that this fix solve my issue !

Thanks for the quick reply !

fabpot added a commit that referenced this issue Jun 15, 2024
…abbuh)

This PR was merged into the 6.4 branch.

Discussion
----------

[Notifier]  send the recipient phone number as an array

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #57393
| License       | MIT

Commits
-------

86480fe send the recipient phone number as an array
@fabpot fabpot closed this as completed Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants