Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-10245

Messenger uses output buffering for error collection, should use error collector instead

    Details

    • Type: Improvement
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.9-RC2
    • Fix Version/s: 3.0.10-RC1
    • Component/s: None
    • Labels:
      None

      Description

      https://github.com/phpbb/phpbb3/blob/develop-olympus/phpBB/includes/functions_messenger.php#L1617

      ob_start();
      // On some PHP Versions mail() *may* fail if there are newlines within the subject.
      // Newlines are used as a delimiter for lines in mail_encode() according to RFC 2045 section 6.8.
      // Because PHP can't decide what is wanted we revert back to the non-RFC-compliant way of separating by one space (Use '' as parameter to mail_encode() results in SPACE used)
      $result = $config['email_function_name']($to, mail_encode($subject, ''), wordwrap(utf8_wordwrap($msg), 997, "\n", true), $headers);
      $err_msg = ob_get_clean();
      

      We now have an error collector class which should be used instead of output buffering and errors going through our error collector.

      In 3.0.9-RC2, msg_handler performs ob_flush which 1) sends output to browser before headers are sent, creating warnings, and 2) results in ob_get_clean not getting any of the output, thus defeating the entire purpose of this code.

      For 3.0.9-RC3 msg_handler will no longer flush, but the correct way to collect errors is to use the error collector.

        Issue Links

          Activity

          Hide
          bantu Andreas Fischer added a comment -

          define('IN_PHPBB', true);
          $phpbb_root_path = (defined('PHPBB_ROOT_PATH')) ? PHPBB_ROOT_PATH : './';
          $phpEx = substr(strrchr(__FILE__, '.'), 1);
          include($phpbb_root_path . 'common.' . $phpEx);
           
          $config['smtp_host'] = 'localhost';
          include($phpbb_root_path . 'includes/functions_messenger.' . $phpEx);
          smtpmail(array('dude@example.org'), 'Subject', 'Hello Dude, it is me!', $err_msg);
          echo $err_msg;
          

          produces

          Could not connect to smtp host : 111 : Connection refused
           
          Errno 2: fsockopen(): unable to connect to localhost:25 (Connection refused)
          

          with error collector and

          Could not connect to smtp host : 111 : Connection refused
           
          <b>[phpBB Debug] PHP Warning</b>: in file <b>/includes/functions_messenger.php</b> on line <b>990</b>: <b>fsockopen(): unable to connect to localhost:25 (Connection refused)</b><br /> 
          

          with ob_start/ob_get_clean.

          So I guess we should always include $errfile and $errline like msg_handler() does, regardless of whether DEBUG_EXTRA is set or not.

          Show
          bantu Andreas Fischer added a comment - define('IN_PHPBB', true); $phpbb_root_path = (defined('PHPBB_ROOT_PATH')) ? PHPBB_ROOT_PATH : './'; $phpEx = substr(strrchr(__FILE__, '.'), 1); include($phpbb_root_path . 'common.' . $phpEx);   $config['smtp_host'] = 'localhost'; include($phpbb_root_path . 'includes/functions_messenger.' . $phpEx); smtpmail(array('dude@example.org'), 'Subject', 'Hello Dude, it is me!', $err_msg); echo $err_msg; produces Could not connect to smtp host : 111 : Connection refused   Errno 2: fsockopen(): unable to connect to localhost:25 (Connection refused) with error collector and Could not connect to smtp host : 111 : Connection refused   <b>[phpBB Debug] PHP Warning</b>: in file <b>/includes/functions_messenger.php</b> on line <b>990</b>: <b>fsockopen(): unable to connect to localhost:25 (Connection refused)</b><br /> with ob_start/ob_get_clean. So I guess we should always include $errfile and $errline like msg_handler() does, regardless of whether DEBUG_EXTRA is set or not.

            People

            • Assignee:
              bantu Andreas Fischer
              Reporter:
              Oleg Oleg [X] (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development