STW: add checkbox to subscribe to newsletter #1061

Yhdistetty
max.mehl yhdistetty 6 committia lähteestä max.mehl/fsfe-website:feature/stw-submails-nl kohteeseen master 2019-09-12 17:00:57 +00:00
Omistaja

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 muutti otsikon STW: add checkbox to subscribe to newsletter otsikoksi WIP: STW: add checkbox to subscribe to newsletter 2019-09-06 12:44:02 +00:00
Tekijä
Omistaja

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 muutti otsikon WIP: STW: add checkbox to subscribe to newsletter otsikoksi STW: add checkbox to subscribe to newsletter 2019-09-06 14:16:30 +00:00
Jäsen

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?
Jäsen

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`.
Tekijä
Omistaja

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 :)
Jäsen

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 :)
Tekijä
Omistaja

I like, thanks! That's a good approach, will try to implement it soon :)

I like, thanks! That's a good approach, will try to implement it soon :)
Tekijä
Omistaja

Implemented. I guess it's fine for now. Will merge and test with the live system

Implemented. I guess it's fine for now. Will merge and test with the live system
max.mehl closed this pull request 2019-09-12 17:00:56 +00:00
Sign in to join this conversation.
No description provided.