STW: add checkbox to subscribe to newsletter #1061

2019-09-06T12:43:50Z

max.mehl commented

2019-09-06 12:43:50 +00:00

I've finally managed to let people ordering promo material subscribe to our newsletter. The problem was caused by Mailman's anti-bot measurements, and them accepting the generated CSRF token only after a few seconds.

Missing so far:

~~the script subscribes user to English newsletter only. We could make this depending on the used language~~

I've finally managed to let people ordering promo material subscribe to our newsletter. The problem was caused by Mailman's anti-bot measurements, and them accepting the generated CSRF token only after a few seconds. Missing so far: * ~~the script subscribes user to English newsletter only. We could make this depending on the used language~~

max.mehl marked the pull request as work in progress 2019-09-06 12:44:02 +00:00

max.mehl commented

2019-09-06 14:16:22 +00:00

Language detection also working, at least from those pages which are actually translated.

Ready for review :)

Language detection also working, at least from those pages which are actually translated. Ready for review :)

max.mehl marked the pull request as ready for review 2019-09-06 14:16:30 +00:00

mweimann commented

2019-09-09 20:33:53 +00:00

I'm not allowed to push into your branch. Maybe next time open the PR in the fsfe-website repo.

Here is the promotion curl code without exec:

# send information to mail-signup.php if user wished to sign up to community mails or newsletter
# This happens in the background because subscription to the NL takes some seconds
function mail_signup($data) {
  $url = $_SERVER['REQUEST_SCHEME']. '://' . $_SERVER['HTTP_HOST'] . '/cgi-bin/mail-signup.php';
  $data = http_build_query($data);

  $curlHandle = curl_init();

  if ($curlHandle === false) {
    // error handling; e.g. logging
  }

  curl_setopt($curlHandle, CURLOPT_URL, $url);
  curl_setopt($curlHandle, CURLOPT_POST, 1);
  curl_setopt($curlHandle, CURLOPT_POSTFIELDS, $data);
  // disables echoing the response
  curl_setopt($curlHandle, CURLOPT_RETURNTRANSFER, true);
  $status = curl_getinfo($curlHandle, CURLINFO_HTTP_CODE);

  if ($status !== 200) {
    // non 200 response handling
  }

  curl_close ($curlHandle);

  // previous exec code
  // exec("curl -d \"$data\" -X POST $url -m 5 > /dev/null &");
}

Depending on the PHP version on the server some options may differ. @max.mehl which version do we have there?

I'm not allowed to push into your branch. Maybe next time open the PR in the fsfe-website repo. Here is the promotion curl code without exec: ```php # send information to mail-signup.php if user wished to sign up to community mails or newsletter # This happens in the background because subscription to the NL takes some seconds function mail_signup($data) { $url = $_SERVER['REQUEST_SCHEME']. '://' . $_SERVER['HTTP_HOST'] . '/cgi-bin/mail-signup.php'; $data = http_build_query($data); $curlHandle = curl_init(); if ($curlHandle === false) { // error handling; e.g. logging } curl_setopt($curlHandle, CURLOPT_URL, $url); curl_setopt($curlHandle, CURLOPT_POST, 1); curl_setopt($curlHandle, CURLOPT_POSTFIELDS, $data); // disables echoing the response curl_setopt($curlHandle, CURLOPT_RETURNTRANSFER, true); $status = curl_getinfo($curlHandle, CURLINFO_HTTP_CODE); if ($status !== 200) { // non 200 response handling } curl_close ($curlHandle); // previous exec code // exec("curl -d \"$data\" -X POST $url -m 5 > /dev/null &"); } ``` Depending on the PHP version on the server some options may differ. @max.mehl which version do we have there?

mweimann commented

2019-09-09 20:43:46 +00:00

And here the suggestion for the second exec:

$cmd = sprintf(
  '%s %s %s %s %s %s',
  $odtfill,
  $template,
  $outfile,
  escapeshellarg('Name=' . $name),
  escapeshellarg('Address=' . $address),
  escapeshellarg('Name=' . $name)
);
shell_exec($cmd);

// previous code
// shell_exec("$odtfill $template $outfile Name=$name Address=$address Name=$name");

Simple golden rule: do not pass anything not under you control unescaped as exec param. Like the name parts coming from $_POST.

And here the suggestion for the second exec: ```php $cmd = sprintf( '%s %s %s %s %s %s', $odtfill, $template, $outfile, escapeshellarg('Name=' . $name), escapeshellarg('Address=' . $address), escapeshellarg('Name=' . $name) ); shell_exec($cmd); // previous code // shell_exec("$odtfill $template $outfile Name=$name Address=$address Name=$name"); ``` Simple golden rule: do not pass anything not under you control unescaped as exec param. Like the name parts coming from `$_POST`.

max.mehl commented

2019-09-10 15:00:36 +00:00

Thank you for the review!

I modified the curl request a bit because it takes quite some time to complete if the newsletter option is requested. So I set a very small timeout.

Regarding odtfill, $name and $address already have been escaped before, but I'll take your code because it is more clean.

Please feel free to review again :)

Thank you for the review! I modified the curl request a bit because it takes quite some time to complete if the newsletter option is requested. So I set a very small timeout. Regarding odtfill, $name and $address already have been escaped before, but I'll take your code because it is more clean. Please feel free to review again :)

mweimann commented

2019-09-11 18:15:05 +00:00

I crawled the PHP doc https://www.php.net/manual/en/function.exec.php and found

Note: If a program is started with this function, in order for it to continue running in the background, the output of the program must be redirected to a file or another output stream. Failing to do so will cause PHP to hang until the execution of the program ends.

→ exec should work for background tasks

In the end it calls another PHP script in the same directory. So why curl anyway? :)

promotion.php

/**
 * Calls the "mail-signup" script with the data.
 * 
 * Sends the script into the background to
 * handle the request asynchronously.
 * 
 * @param array $data
 * @see mail-signup.php
 */
function mailSignUp(array $data) {
  $cmd = sprintf(
    'php %s %s > /dev/null &',
    __DIR__ . '/mail-signup.php',
    escapeshellarg(json_encode($data))
  );
  exec($cmd);
}

mail-signup.php

// parse data from POST or cli arg

if (php_sapi_name() === 'cli') {
  $data = json_decode($argv[1], true);
} else {
  $data = $_POST;
}

// Available params: list, mail, name, language, address, zip, city, country

$list = $data['list'] ?? false;
$mail = $data['mail'] ?? false;
$name = $data['name'] ?? false;
$lang = $data['lang'] ?? false;
$address = $data['address'] ?? false;
$zip = $data['zip'] ?? false;
$city = $data['city'] ?? false;
$country = $data['country'] ?? false;

I somehow can't test that in my environment. Maybe we can work having that running in my docker setup when we meet next time :)

I crawled the PHP doc https://www.php.net/manual/en/function.exec.php and found > Note: If a program is started with this function, in order for it to continue running in the background, the output of the program must be redirected to a file or another output stream. Failing to do so will cause PHP to hang until the execution of the program ends. → exec should work for background tasks In the end it calls another PHP script in the same directory. So why curl anyway? :) promotion.php ```php /** * Calls the "mail-signup" script with the data. * * Sends the script into the background to * handle the request asynchronously. * * @param array $data * @see mail-signup.php */ function mailSignUp(array $data) { $cmd = sprintf( 'php %s %s > /dev/null &', __DIR__ . '/mail-signup.php', escapeshellarg(json_encode($data)) ); exec($cmd); } ``` mail-signup.php ```php // parse data from POST or cli arg if (php_sapi_name() === 'cli') { $data = json_decode($argv[1], true); } else { $data = $_POST; } // Available params: list, mail, name, language, address, zip, city, country $list = $data['list'] ?? false; $mail = $data['mail'] ?? false; $name = $data['name'] ?? false; $lang = $data['lang'] ?? false; $address = $data['address'] ?? false; $zip = $data['zip'] ?? false; $city = $data['city'] ?? false; $country = $data['country'] ?? false; ``` I somehow can't test that in my environment. Maybe we can work having that running in my docker setup when we meet next time :)

max.mehl commented

2019-09-12 08:45:10 +00:00