#1061 STW: add checkbox to subscribe to newsletter

Merged
max.mehl merged 6 commits from max.mehl:feature/stw-submails-nl into master 1 month ago
max.mehl commented 1 month ago

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 changed title from STW: add checkbox to subscribe to newsletter to WIP: STW: add checkbox to subscribe to newsletter 1 month ago
max.mehl commented 1 month ago
Owner

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 changed title from WIP: STW: add checkbox to subscribe to newsletter to STW: add checkbox to subscribe to newsletter 1 month ago
mweimann commented 1 month ago
Collaborator

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 1 month ago
Collaborator

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 1 month ago
Owner

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 1 month ago
Collaborator

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 1 month ago
Owner

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 :)
max.mehl commented 1 month ago
Owner

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
The pull request has been merged.
Sign in to join this conversation.
Loading…
Cancel
Save
There is no content yet.