Hi All

In some circumstances (e.g. when using simplenews and mimemail) the _drupal_html_to_text_pad helper function called by drupal_html_to_text can try to pad headers with negative padding.

This can happen when using nodereferences to pull teasers into a newsletter.

For example:

print drupal_html_to_text('<h2>This is an extremely long title for a node http://www.example.com/news/this-is-an-extremely-long-title-for-a-node </h2>');

This patch stops the padding length from falling below zero.

Cheers

CommentFileSizeAuthor
#6 mail.inc_.padding-508738-6.patch732 byteseric_a
mail.inc_.padding.patch537 bytesterrychild

Comments

RAguilar’s picture

RAguilar commented

what about something like this

line:480
$value = (($n - strlen($prefix))>=0)? ($n - strlen($prefix)) : 0;
return $text . $prefix . str_repeat($pad, $value) ."\n";

this prevents from falling below zero but i dont know if this is the best solution

paulrooney’s picture

paulrooney commented

This patch worked great and resolved PHP errors when sending HTML mail with Mime Mail 6.x-1.0-alpha1 and Simplenews 6.x-1.0-rc6

The error message was as follows:
str_repeat() [function.str-repeat]: Second argument has to be greater than or equal to 0 in /home/[username]/drupal-6.14/includes/mail.inc on line 480.

Thanks!

Delta Bridges’s picture

Delta Bridges commented

I am having a similar error when sending newsletter with mime mail and simplenews:

str_repeat() [function.str-repeat]: Second argument has to be greater than or equal to 0 in /home/xxx/public_html/xxx/includes/mail.inc on line 480.

Will this patch be implemented in a future version?

Many thanks

eric_a’s picture

eric_a commented
Version: 6.x-dev » 7.x-dev

If the issue is with _drupal_html_to_text_pad() then move to 7.x to increase chances on getting some attention. The only difference between 7.x version and 6.x is coding style.

Status: Needs review » Needs work

The last submitted patch, mail.inc_.padding.patch, failed testing.

eric_a’s picture

eric_a commented
Status: Needs work » Needs review
StatusFileSize
new mail.inc_.padding-508738-6.patch732 bytes

Reroll.

eric_a’s picture

eric_a commented
Priority: Normal » Critical

This bug blocks a decent test #299138: Improve \Drupal\Core\Utility\Mail::htmlToText() and gives us PHP warnings and messed up e-mail bodies.

catch’s picture

catch
he/him
Primary language English
commented
Priority: Critical » Normal

Blocking a test doesn't qualify this as a release blocker.

eric_a’s picture

eric_a commented
Status: Needs review » Needs work

With the patch we are always lowering the repeat multiplyer, i.e. changing the behaviour even for the cases that did not produce an error. The suggestion from Raguilar in #1 only changes the multiplier if it would fall below zero, which is in line with the goal of this issue.

eric_a’s picture

eric_a commented
Status: Needs work » Reviewed & tested by the community

OMG, I can't believe I said that (#9). It is totally wrong.

Raguilar had the same idea and basically the same code as terrychild and the code seems fine to me, too.

dries’s picture

dries commented
Version: 7.x-dev » 6.x-dev

Committed to CVS HEAD. Moving to Drupal 6.

eric_a’s picture

eric_a commented

Patch in OP #1 still applies perfectly to DRUPAL-6.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.