You are here: Foswiki>Tasks Web>Item10475 (05 Jul 2015, GeorgeClark)Edit Attach

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

pencil
Priority: Normal
Current State: Closed
Released In: 2.0.0
Target Release: major
Applies To: Engine
Component: PlainFileStoreContrib, RCSStoreContrib, core
Branches: trunk
Reported By: SvenDowideit
Waiting For:
Last Change By: GeorgeClark
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

 
Topic revision: r11 - 05 Jul 2015, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy