Never trust your data

Nov 02

von Sebastian am 2.11.2012 um 18:08 in English, Perl

Dieser Post wurde aus meiner alten WordPress-Installation importiert. Sollte es Darstellungsprobleme, falsche Links oder fehlende Bilder geben, bitte einfach hier einen Kommentar hinterlassen. Danke.

Warnings aren't useless, they may be hints to invisible problems - which could become "big trouble" lateron. Recently, I was trying to track down a warning which didn't make any sense.

substr outside of string at My/Module.pm line 425.

Easy to fix, isn't it? But unfortunately not here.

My sub is walking through a string in a loop and shouldn't run a length() call on the string for every single loop iteration. Easy solution: Run the length() call once and store the result in a variable - a typical memory-to-speed conversion.

Another variable will hold the current position within the string, a for my $pos (1..length($string)) won't work, because the script processes the string in variable sized blocks.

my $strlen = length($string);my $pos = 0;while ($pos < $strlen) {

The loop source adds something to $pos in every iteration (the loop would be endless otherwise), depending on the string itself.

Here is the complaining line 425 and the two lines above:

 my $num_len = $strlen - $pos; $num_len = 10 if $num_len > 10; my $part = substr($string, $pos, $num_len);

I changed it from an unconditional substr call to an exact one using only the chars left in the string (or less).

But the warning is still firing - why? I tried many samples, here is one: $strlen = 40, $pos = 30, resulting $num_len will be 10 - which is correct.

It took a long time until I replayed a complete run (which isn't easy for this module) with many debug statements (prints) and it turned out, that

$strlen was 40 (which sounds good for the test case)
$pos was 152 (which is impossible if $strlen is 40)
$num_len was -112 (which is correct math, but invalid for this substr call here, because it should never get any chars from the end of the string)

The block length for each part of each iteration depends on the $string data itself - it has a field length stored within the merged data - and the incoming data was invalid. Valid enough to pass all checks and reach the loop (I didn't expect this to be possible at all) but failing within the loop (resulting in an overall-fail of the sub which is the expected behavior for invalid data).

I added a simple check after every $pos change: