perl-pod / pod-simple

Framework for Parsing and Formatting POD
http://search.cpan.org/dist/Pod-Simple/
44 stars 60 forks source link

Pod::Simple::XHTML::encode_entities: how should an uninitialized value be handled? #135

Open jkeenan opened 3 years ago

jkeenan commented 3 years ago

Pod::Simple::XHTML::encode_entities() is not a publicly documented method in its class, but it is invoked within several of the class's methods that are publicly documented. In certain situations, use of this method will throw an uninitialized value ... in substitution warning.

Here is the method's definition:

 64 sub encode_entities {
 65   my $self = shift;
 66   my $ents = $self->html_encode_chars;
 67   return HTML::Entities::encode_entities( $_[0], $ents ) if $HAS_HTML_ENTITIES;
 68   if (defined $ents) {
 69       $ents =~ s,(?<!\\)([]/]),\\$1,g;
 70       $ents =~ s,(?<!\\)\\\z,\\\\,;
 71   } else {
 72       $ents = join '', keys %entities;
 73   }
 74   my $str = $_[0];
 75   $str =~ s/([$ents])/'&' . ($entities{$1} || sprintf '#x%X', ord $1) . ';'/ge;
 76   return $str;
 77 }

Note that after line 74 there is no check to ensure that $str is defined. While debugging a problem installhtml in the Perl 5 core distribution today, I got these warnings:

$ grep -n uninitialized installhtml.c94c9a62ab.2.log
262:./installhtml: Warning both '/home/jkeenan/gitwork/perl//dist/Locale-Maketext/lib/Locale/Maketext.pod' and '/home/jkeenan/gitwork/perl//dist/Locale-MaketextUse of uninitialized value $str in substitution (s///) at lib/Pod/Simple/XHTML.pm line 75.
667:./installhtml: Warning both '/home/jkeenan/gitwork/perl//cpan/Pod-Simple/lib/Pod/Simple.pod' and '/home/jkeenan/gitwork/perl//cpan/Pod-Simple/lib/Pod/Simple.pm' exist, using poUse of uninitialized value $str in substitution (s///) at lib/Pod/Simple/XHTML.pm line 75.

(Ignore the Warning both part of the strings. What I'm reporting is the Use of uninitialized value $str in substitution (s///) part.)

It's not yet clear to me what the function should do if it gets as far as line 74 but $_[0] is undefined.

haarg commented 3 years ago

I think the current behavior is fine. The method expects to be given a string. If you pass undef, it coerces it to an empty string and throws a warning, just like most string operations in perl.