Working with a product today (not mentioning names), I discovered the output from one of it's provided 'helpers' was not as documented, with additional backslashes appearing in the output where none occured in the input. A fair assumption then that this program was attempting to escape the values.
Diving into the code I discovered a likely clause which first struck me as being.
- Rather queer expression of logic
- Broken - since the regex was not escaping things correctly
- TRUE if $val has any double-quotes or backslashes , replace them with a backslash and the empty match value $1
- OR if $val contains a colon or white space
if ($val =~ s/["\\]/\\$1/g or $val =~ m/[:\s]/) {
$val = qq["$val"];
}
My biggest gripe with this fragment of code is the mutating of $val in the condition. It seems very wrong to me. As a quick fix I modified the substitution regex to what I believed was intended. if ( $val =~ s/(["\\])/\\$1/g or ..... Excellent, my short test program is now escaping things rather than replacing double-quotes with backslashes. Let's apply these changes to the vendor code - it appears in two places so I change them both.
No luck, some of the output is now correct, but mysteriously , some is not. Uh oh. The quoting is correct but some is quoted, the quotes escaped , then quoted again. I knew that mutator was evil.
"this was quoted because of whitespace" "\"this was quoted because of whitespace\""
Why Why Why! - I'll tell you why , $val is a scalar stored in an array. A reference to this array is used twice, in a foreach like so
foreach my $val ( @{$arrayref} ) {
# do evil mutating things to $val
}
foreach my $val ( @{$arrayref} ) {
# now do it AGAIN.
}
A quick smattering of warns confirms the bad news
FIRST LOOP SCALAR(0x857c5cc) SCALAR(0x857c5d8) SCALAR(0x857c5e4) SCALAR(0x857c5f0) SECOND LOOP SCALAR(0x857c5cc) SCALAR(0x857c5d8) SCALAR(0x857c5e4) SCALAR(0x857c5f0)And that's where the story ends. I'd be intrigued to hear other peoples experiences of being ambushed by a bigger bug whilst fixing another.
I can't believe it's not psellchecked
I was once charged with maintaining a piece of Perl code that was responsible for parsing two CSV files, extracting and combining interesting information, and writing out a new "result" CSV.
The original author had made some assumptions about the CSV format that were simplistic (and he hadn't used Text::CSV_XS or it's compatriots). So, when the application that generated the two source CSV files found commas in the data for the first time, it did the right thing and quoted the whole value; the code died.
I set out to fix this bug by replacing the homegrown CSV parser with Text::CSV_XS. In the process, however, I discovered a bug in the result-set generator that had been skipping important information for several years -- I'd never have noticed if I hadn't constructed test data with which to test my changes. The "2-hour bugfix" I'd pitched to management turned into a two-week re-write.
It was that experience which started me down the road to test-driven development...
A collection of thoughts and links from the minds of geeks
The Code that can be seen is not the true Code
I haven't found a problem yet that can't be solved by a well-placed trebuchet
I came back to finish a project I'd put down 18 months previous; usually such a long delay means I must spend an hour or two reestablishing context (and afterwards, constantly stumbling across the things I hadn't quite finished but hadn't quite documented for myself to find later; also known as "bugs").
But this one I had developed using TDD; I ran the unit test and it listed before my eyes everything I'd worked on, the order I'd done it, and thus reestablished my work flow in about 45 seconds. The very last test was failing, so I knew precisely what I had been working on when I put the project down a year and a half ago.
I was making forward progress in under two minutes. World record for me.
I love TDD.