Item10475: Foswiki::Func::readAttachment( $web, $topic, undef ); returns the topic text.

this is going to make code paths more complex, isn't what we docco, and doesnt' make much sense to the user.

working on trunk atm, might be a 1.1 thing, or even a 1.0?

-- SvenDowideit - 12 Mar 2011

It appears as though the issue is in the interface between Meta and Store. Nothing asserts that the Attachment is not undef.

• Func calls Meta openAttachment
  • Meta calls Store openAttachment
    • Store requests getHandler for the topic / attachment
      • getHandler creates a new Handler for web / topic / attachment.

And there is the issue, as the way to get a handler for the topic is to pass an undefined attachment. So store is working correctly, returning a handler for the topic if attachment is undef. I'm guessing that Meta should throw an error instead of request a handler for an undef attachment, and they both ought to assert if DEBUG is enabled.

Crawford, could you give this some thought. I wasn't sure the best way to handle, and how deep the checks should go. I've confirmed this in the StoreTests.

my $fh = $this->{test_topicObject}->openAttachment( undef, '<' );

returns a file handle for the topic instead of the attachment.

• Func uses _validateWTA that will "die" if an invalid attachment name is passed, but it doesn't if the attachment starts out as undef. So one possible fix would be to just die if Func::readAttachment is called with an undef attachment name.
• Another option is to ASSERT if Func::readAttachment doesn't name an attachment.
• Store openAttachment doesn't validate the attachment name. So maybe add an ASSERT( $attachment ) if DEBUG; to Store.pm

-- GeorgeClark - 19 Jun 2014

The store API definition hands off responsibility for checking the topic to the caller, and needs to do the same with the attachment for best DRY. All callers of the store need to make sure they are passing the right thing; attachment names cannot evaluate to perl false. Yes, there needs to be an ASSERT, but in each Store implementation.

Reassigned to FoswikiStore, UnitTestContrib, RCSStoreContrib and PlainFileStoreContrib.

Added appropriate asserts and tests.

-- CrawfordCurrie - 28 Jun 2014

Commenting is disabled while not logged in