Alfahosting SMS

Hello,

thank you for reviewing.

About the branches: The code in both branches is the same, because I use only hooks provided by the SMS Framework and there is no difference between the Drupal major versions. The difference is only in the core parameter of the sms_alfahosting.info file.

About the t() usage:
I am using @varname in all messages used with t()
The only place that is looks strange at the first look is the line 80. t($response['message'], $response['variables'])
But this is because I am using there the return of a function that returns an array for the SMS Framework.
Here are example values of this variables from the line 154:

$response = array('status' => FALSE, 'message' => 'The Gateway respond with an error: @errno - @error', 'variables' => array('@errno' => intval($xml_object->error->code), '@error' => strval($xml_object->error->message)));

About the security issue
I pass the values of the admin form directly into the XML building function of the API request. In that line I pass them to make a fake request to validate all the parameters with an example/fake request to the API. The settings will be saved only if the API successfully validates these parameters and responds with a success message like this:

<?xml version="1.0"?>
<alfasms><success><message>SMS would have been sent; aborted due to fake mode.</message></success></alfasms>

I haven't use check_plain() because if there is something wrong with the values of the admin form fields, then the API fake call will fail and the admin form will display an error message and will not save the values to the variables.

For building the XML I am using SimpleXML, so the values of the admin form fields can not brake the XML itself, because they become automatically escaped by SimpleXML if they contain illegal signs.

About the hardcoded number
This is not a real number. It's a fake number used for the validation/fake request to the Gateway API. In this step I am validating the admin form values over an Gateway API fake call. The admin form values are "Username", "Key" and "Sender". The reciver don't need to be validated in this step so I hardcoded this fake number.

Ok. Let's say it is the 6.x-1.x branch (I used D6 and smsframework-6.x-1.0 for the development) ;)

About the user input

check_plain() should be used to filter user entered values, but: In this case the values of the form fields are not displayed in the HTML of Drupal pages (only s #default_value) and are not used in SQL queries.
In the form validation function I pass the values to the XML building function. The XML building function is using SimpleXML to get valid XML. So SimpleXML is escaping everything that needs to be escaped to get a valid XML. After that I send this XML to the Gateway API for validation.
So if there are some illegal characters transferred to the API, the validation will fail and they will not be saved.
To get the values to be saved to the variable the admin of the site need to enter a valid username, valid key (the key can contain many special characters like <>,'`´$%&) and a valid sender.

I analyzed many cases and didn't found a way to pass the validation with invalid data or manipulate the validation process.

I pushed everything found by DrupalCS and added an inline comment to the validation.

About the t() usage
This module is a submodule for the SMS Framework module and the return value of the sms_alfahosting_process_request() function is used as the return value of the sms_alfahosting_send_sms() callback function. Regarding to the SMS Framework documentation (Create the custom send function) :

• The callback should return an array indicating the result of the send.
• status - Set to TRUE if the send was successful, FALSE if it was not.
• message - If 'status' is FALSE, you may provide more details about the error.
  See t() for documentation on how 'message' and 'variables' interact. Keep 'message' translatable by not concatenating dynamic values into it!
• variables - Array of variables to replace in the message or NULL if message is already translated or not possible to translate.

So I can't use t(), because the SMS Framework module will use it again.