CGI.pm uploading bugs fixed

markjugg on 2007-04-17T16:04:28

Some years ago, I noticed an oddity when using CGI.pm file uploads. File uploading only worked on first object I created. If I called CGI->new() again, the file upload would be gone.

At the time, I considered file uploading to a difficult and mysterious area, and assumed it "had to be this way", so I didn't file a bug report, and learned to work around the issue.

"CGI.pm is very popular" I reasoned. "Therefore, obvious bugs in it would be spotted and repaired."

For years, I avoided the problem by using CGI::Application and related plugins. It loads CGI.pm just once and provides access to it through a "query" method. So, as long as all the plugins use this method and don't access CGI.pm directly, the bug can continue avoided.

Finally, however, it bit me again recently in a way that was very hard to track down. Suddenly, we were getting weird empty files. This seemed impossible, as the files clearly existed and were being validated by Data::FormValidator::Constraints::Upload.

The files were "disappearing" because they weren't available on a second call to CGI->new.

This time, I had more experience and a different attitude. I decided this might be a solvable bug, and I dove into the CGI.pm guts to investigate.

This time around, I also had an understanding how to write an automated test for a file upload, which had also been a mystery in the past.

The result was that I was able to verify the issue was real, *and* solvable, and those fixes are what's new in CGI.pm 3.29:

http://search.cpan.org/src/LDS/CGI.pm-3.29/Changes

The moral, though, is to not settle for working around weird, mysterious behavior. At least report it to a user's mailing list for the software, or be adventuresome and dive into the code and poke around.

The fix for the issue was not particular hard and the symptoms were certainly obvious to those who ran into it. I can only conclude that the bug existed for years unfixed because other people like me had an attitude that the issue was "just mysterious" or couldn't be fixed.


Good work!

jdavidb on 2007-04-17T19:23:23

Awesome work! I hope I someday have the boldness to plunge into code like Net::SFTP::Foreign and fix bugs I've found. :)

BTW, an entry on how you did the test would be interesting to people like me, who have a vague idea how they might go about such a test but still probably regard it as black magic. Is this a test that's going to run every time I and every other installer of this module do a make test? Is it something that's going to fail if I'm not connected to the net (or required to access the web through a proxy)?

how to test file uploads

markjugg on 2007-04-18T02:33:33

You can look at t/upload.t in the CGI.pm distribution to see how the test works. The crux of it is that I reverse engineered what the environment and "POST" contents of a file upload looked like, and then I faked it so that a call to CGI->new() thinks a real file upload is happening.

If you want to test a file upload in a private project, that's a lot of trouble to go through. I would recommend Test::WWW::Mechanize::CGI instead.

That didn't exist when I was first figuring out how to test file uploads, otherwise I probably never would have figured out the technique. The only benefit of directly mocking-up the POST is that you don't have all the dependencies of Test::WWW::Mechanize::CGI. That whole dependency chain would be a pain to ship with CGI.pm just for a few tests.

    Mark

Good Work!

Shlomi Fish on 2007-04-22T20:32:33

Good work! I think it's always preferable to fix such problems (at least if the source is available and you are allowed to modify it) rather than work around it.