Issue Details (XML | Word | Printable)

Key: MDL-18368
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Francois Marier
Reporter: Anthony Borrow
Votes: 0
Watchers: 0
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle

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

Created: 25/Feb/09 09:43 AM   Updated: 03/Apr/09 10:38 AM
Return to search
Component/s: Lib
Affects Version/s: 1.9.4
Fix Version/s: 1.8.9, 1.9.5

File Attachments: 1. File html2text.diff (0.9 kB)

Issue Links:
Dependency
 

Participants: Anthony Borrow and Francois Marier
Security Level: None
Resolved date: 03/Apr/09
Affected Branches: MOODLE_19_STABLE
Fixed Branches: MOODLE_18_STABLE, MOODLE_19_STABLE


 Description  « Hide
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

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Anthony Borrow added a comment - 25/Feb/09 09:51 AM
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


Anthony Borrow added a comment - 25/Feb/09 02:44 PM
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

Anthony Borrow added a comment - 25/Feb/09 03:09 PM
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

Anthony Borrow added a comment - 25/Feb/09 03:10 PM
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

Anthony Borrow added a comment - 25/Feb/09 03:12 PM
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

Francois Marier added a comment - 03/Apr/09 07:18 AM
Committed a fix on 1.8 and 1.9 stable as part of MDL-17542.

Anthony Borrow added a comment - 03/Apr/09 10:38 AM
marking as closed since it was committed and I already tested the patch