Moodle

html2text.php - notice when given a blank/empty string

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9.4
  • Fix Version/s: 1.8.9, 1.9.5
  • Component/s: Libraries
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE

Description

I was working with some contrib code which was sending an empty value to html2text.php. As a result, the following notice was produced:

Notice: Uninitialized string offset: 0 in /home/arborrow/Moodle/code/19stable/lib/html2text.php on line 39

I am attaching a diff file that gets around the notice; however, I'm not sure if setting $chr to an empty string is the best thing. But I think verifying that there is something there to work with in $badStr{0} is a good idea.

Peace - Anthony

Issue Links

This issue will be resolved by:
MDL-17542 html2text appears to be incompatible with the GPL Major Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Anthony Borrow added a comment -

A couple of questions that came to mind as I was looking at this.

1) Why is html2text.php function its own file? Is it just for licensing? Any reason not to include into a standard library (weblib or some other).

2) I generally like to see a default value in the case that one is not provided. Would there be any advantage to setting $badStr = '' or NULL? Similarly in /lib/weblib.php with the html_to_text($html) declaration any advantage to declaring a default value.

Peace - Anthony

Show
Anthony Borrow added a comment - A couple of questions that came to mind as I was looking at this. 1) Why is html2text.php function its own file? Is it just for licensing? Any reason not to include into a standard library (weblib or some other). 2) I generally like to see a default value in the case that one is not provided. Would there be any advantage to setting $badStr = '' or NULL? Similarly in /lib/weblib.php with the html_to_text($html) declaration any advantage to declaring a default value. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

I just noticed that the diff file makes no difference. I had read the badStr{0} as badStr[0] so ignore the diff file since it does not fix the notice. Peace - Anthony

Show
Anthony Borrow added a comment - I just noticed that the diff file makes no difference. I had read the badStr{0} as badStr[0] so ignore the diff file since it does not fix the notice. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

I am thinking $badStr{0} was a typo and that it should have been $badStr[0]. In any case, I discovered that if I apply the patch for 1.9 listed in MDL-17542 that I do not get a notice. I will mark that resolving MDL-17542 will resolve this issue. Peace - Anthony

Show
Anthony Borrow added a comment - I am thinking $badStr{0} was a typo and that it should have been $badStr[0]. In any case, I discovered that if I apply the patch for 1.9 listed in MDL-17542 that I do not get a notice. I will mark that resolving MDL-17542 will resolve this issue. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

Francois - I am assigning this to you as I think it is ultimately related to MDL-17542. Let me know if you have any questions. If we are able to apply the patches to 1.9 and 1.8 listed in MDL-17542, I think this issue could be resolved as Not a bug. Peace - Anthony

Show
Anthony Borrow added a comment - Francois - I am assigning this to you as I think it is ultimately related to MDL-17542. Let me know if you have any questions. If we are able to apply the patches to 1.9 and 1.8 listed in MDL-17542, I think this issue could be resolved as Not a bug. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

The new code not only resolved the licensing issue and is faster but it also avoid producing notices in certain situations (namely when no value is passed to it). I would vote for MDL-17542 being applied to 19STABLE and 18STABLE. Peace - Anthony

Show
Anthony Borrow added a comment - The new code not only resolved the licensing issue and is faster but it also avoid producing notices in certain situations (namely when no value is passed to it). I would vote for MDL-17542 being applied to 19STABLE and 18STABLE. Peace - Anthony
Hide
Permalink
Francois Marier added a comment -

Committed a fix on 1.8 and 1.9 stable as part of MDL-17542.

Show
Francois Marier added a comment - Committed a fix on 1.8 and 1.9 stable as part of MDL-17542.
Hide
Permalink
Anthony Borrow added a comment -

marking as closed since it was committed and I already tested the patch

Show
Anthony Borrow added a comment - marking as closed since it was committed and I already tested the patch

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.