Changeset 4606

Timestamp:

08/06/09 08:32:27 (3 years ago)

Author:

CrawfordCurrie

Message:

Item935: the access rights that were checked are those expressed in the *new* topic text, instead of the old. Took the opportunity to deprecate the crappy readTopicText and saveTopicText interfaces while I was in there.

Location:

trunk

Files:

• UnitTestContrib/test/unit/FuncTests.pm (modified) (1 diff)
• core/lib/Foswiki/Func.pm (modified) (7 diffs)

trunk/UnitTestContrib/test/unit/FuncTests.pm

@@ -155 +155 @@
     $this->assert( !$login );
     $this->assert_equals( 0, $time );
+}
+
+# As much as I'd like to remove this dumb function, make sure it's still
+# compatible
+sub test_saveTopicText {
+    my $this = shift;
+    my $topic = 'SaveTopicText';
+    Foswiki::Func::saveTopicText(
+        $this->{test_web}, $topic, <<NONNY );
+   * Set ALLOWTOPICCHANGE = NotMeNoNotMe
+NONNY
+    $this->assert(!
+      Foswiki::Func::checkAccessPermission( 'CHANGE',
+        Foswiki::Func::getWikiName(), undef, $topic, $this->{test_web} ));
+
+    # This should fail and return an oopsUrl (FFS, what a shit spec)
+    my $oopsURL = Foswiki::Func::saveTopicText(
+        $this->{test_web}, $topic, 'Gasp' );
+    $this->assert($oopsURL);
+    my @ri = Foswiki::Func::getRevisionInfo($this->{test_web}, $topic);
+    $this->assert_matches(qr/1$/, $ri[2]);
+
+    # This should succeed and return undef
+    $oopsURL = Foswiki::Func::saveTopicText(
+        $this->{test_web}, $topic, 'Beam', 1 );
+    $this->assert(!$oopsURL, $oopsURL);
+}
+
+sub test_saveTopic {
+    my $this = shift;
+    my $topic = 'SaveTopic';
+    Foswiki::Func::saveTopic(
+        $this->{test_web}, $topic, undef, <<NONNY );
+%META:PREFERENCE{name="Bird" value="Kakapo"}%
+   * Set ALLOWTOPICCHANGE = NotMeNoNotMe
+NONNY
+    $this->assert(!
+      Foswiki::Func::checkAccessPermission( 'CHANGE',
+        Foswiki::Func::getWikiName(), undef, $topic, $this->{test_web} ));
+    my @ri = Foswiki::Func::getRevisionInfo($this->{test_web}, $topic);
+    $this->assert_matches(qr/1$/, $ri[2]);
+
+    # Make sure the meta got into the topic
+    my ($m, $t) = Foswiki::Func::readTopic($this->{test_web}, $topic);
+    my $el = $m->get('PREFERENCE', 'Bird');
+    $this->assert_equals('Kakapo', $el->{value});
+
+    # This should succeed
+    Foswiki::Func::saveTopic($this->{test_web}, $topic, undef, 'Gasp',
+                            { forcenewrevision => 1 });
+    @ri = Foswiki::Func::getRevisionInfo($this->{test_web}, $topic);
+    $this->assert_matches(qr/2$/, $ri[2]);
 }
 

trunk/core/lib/Foswiki/Func.pm

@@ -1354 +1354 @@
 </verbatim>
 
-__Note:__ Plugins handlers ( e.g. =beforeSaveHandler= ) will be called as
-appropriate.
+__Note:__ Access controls are *not* checked by this function. You should
+always call =checkAccessPermission= beforehand. Plugins handlers ( e.g.
+=beforeSaveHandler= ) will be called as appropriate.
 
 In the event of an error an exception will be thrown. Callers can elect
 to trap the exceptions thrown, or allow them to propagate to the calling
-environment. May throw Foswiki::OopsException, Foswiki::AccessControlException or Error::Simple.
+environment. May throw Foswiki::OopsException or Error::Simple.
 
 =cut
@@ -1370 +1371 @@
     $topicObject->copyFrom($smeta) if $smeta;
     return $topicObject->save(%$options);
-}
-
-=begin TML
-
----+++ saveTopicText( $web, $topic, $text, $ignorePermissions, $dontNotify ) -> $oopsUrl
-
-Save topic text, typically obtained by readTopicText(). Topic data usually includes meta data; the file attachment meta data is replaced by the meta data from the topic file if it exists.
-   * =$web=                - Web name, e.g. ='Main'=, or empty
-   * =$topic=              - Topic name, e.g. ='MyTopic'=, or ="Main.MyTopic"=
-   * =$text=               - Topic text to save, assumed to include meta data
-   * =$ignorePermissions=  - Set to ="1"= if checkAccessPermission() is already performed and OK
-   * =$dontNotify=         - Set to ="1"= if not to notify users of the change
-Return: =$oopsUrl=               Empty string if OK; the =$oopsUrl= for calling redirectCgiQuery() in case of error
-
-This method is a lot less efficient and much more dangerous than =saveTopic=.
-
-<verbatim>
-my $text = Foswiki::Func::readTopicText( $web, $topic );
-
-# check for oops URL in case of error:
-if( $text =~ /^http.*?\/oops/ ) {
-    Foswiki::Func::redirectCgiQuery( $query, $text );
-    return;
-}
-# do topic text manipulation like:
-$text =~ s/old/new/g;
-# do meta data manipulation like:
-$text =~ s/(META\:FIELD.*?name\=\"TopicClassification\".*?value\=\")[^\"]*/$1BugResolved/;
-$oopsUrl = Foswiki::Func::saveTopicText( $web, $topic, $text ); # save topic text
-</verbatim>
-
-=cut
-
-sub saveTopicText {
-    my ( $web, $topic, $text, $ignorePermissions, $dontNotify ) = @_;
-    ASSERT($Foswiki::Plugins::SESSION) if DEBUG;
-
-    my $session = $Foswiki::Plugins::SESSION;
-
-    # extract meta data and merge old attachment meta data
-    require Foswiki::Meta;
-    my $topicObject = Foswiki::Meta->new( $session, $web, $topic, $text );
-    $topicObject->remove('FILEATTACHMENT');
-
-    my $oldMeta = Foswiki::Meta->load( $session, $web, $topic );
-    $topicObject->copyFrom( $oldMeta, 'FILEATTACHMENT' );
-
-    my $outcome = '';
-    unless ( $ignorePermissions || $topicObject->haveAccess('CHANGE') ) {
-        my @caller = caller();
-        return getScriptUrl(
-            $web, $topic, 'oops',
-            template => 'oopsattention',
-            def      => 'topic_access',
-            param1   => ( $caller[0] || 'unknown' )
-        );
-    }
-
-    try {
-        $topicObject->save( notify => $dontNotify );
-    }
-    catch Error::Simple with {
-        $outcome = getScriptUrl(
-            $web, $topic, 'oops',
-            template => 'oopsattention',
-            def      => 'save_error',
-            param1   => shift->{-text}
-        );
-    };
-    return $outcome;
 }
 
@@ -1580 +1511 @@
     my $meta = Foswiki::Meta->load( $Foswiki::Plugins::SESSION, @_ );
     return ( $meta, $meta->text() );
-}
-
-=begin TML
-
----+++ readTopicText( $web, $topic, $rev, $ignorePermissions ) -> $text
-
-Read topic text, including meta data
-   * =$web=                - Web name, e.g. ='Main'=, or empty
-   * =$topic=              - Topic name, e.g. ='MyTopic'=, or ="Main.MyTopic"=
-   * =$rev=                - Topic revision to read, optional. Specify the minor part of the revision, e.g. ="5"=, not ="1.5"=; the top revision is returned if omitted or empty.
-   * =$ignorePermissions=  - Set to ="1"= if checkAccessPermission() is already performed and OK; an oops URL is returned if user has no permission
-Return: =$text=                  Topic text with embedded meta data; an oops URL for calling redirectCgiQuery() is returned in case of an error
-
-This method is more efficient than =readTopic=, but returns meta-data embedded in the text. Plugins authors must be very careful to avoid damaging meta-data. You are recommended to use readTopic instead, which is a lot safer.
-
-=cut
-
-sub readTopicText {
-    my ( $web, $topic, $rev, $ignorePermissions ) = @_;
-    ASSERT($Foswiki::Plugins::SESSION) if DEBUG;
-
-    my $user;
-    $user = $Foswiki::Plugins::SESSION->{user}
-      unless defined($ignorePermissions);
-
-    my $topicObject =
-      Foswiki::Meta->load( $Foswiki::Plugins::SESSION, $web, $topic, $rev );
-    my $text;
-    if (
-        $topicObject->haveAccess( 'VIEW', $Foswiki::Plugins::SESSION->{user} ) )
-    {
-        $text = $topicObject->getEmbeddedStoreForm();
-    }
-    else {
-        $text = getScriptUrl(
-            $web, $topic, 'oops',
-            template => 'oopsaccessdenied',
-            def      => 'topic_access',
-            param1   => 'VIEW',
-            param2   => $Foswiki::Meta::reason
-        );
-    }
-
-    return $text;
 }
 
@@ -2982 +2869 @@
 ---+++ getDataDir( ) -> $dir
 
-*Deprecated* 28 Nov 2008 - use the "Webs, Topics and Attachments" functions to manipulate topics instead
+*Deprecated* 28 Nov 2008 - use the "Webs, Topics and Attachments" functions
+to manipulate topics instead
 
 =cut
@@ -2994 +2882 @@
 ---+++ getPubDir( ) -> $dir
 
-*Deprecated* 28 Nov 2008 - use the "Webs, Topics and Attachments" functions to manipulateattachments instead
+*Deprecated* 28 Nov 2008 - use the "Webs, Topics and Attachments" functions
+to manipulate attachments instead
 
 =cut
@@ -3004 +2893 @@
 ---+++ getCgiQuery( ) -> $query
 
-*Deprecated* 31 Mar 2009 - use getRequestObject instead.
+*Deprecated* 31 Mar 2009 - use =getRequestObject= instead.
 
 =cut
@@ -3014 +2903 @@
     die
 "checkDependencies removed; contact plugin author or maintainer and tell them to use BuildContrib DEPENDENCIES instead";
+}
+
+
+=begin TML
+
+---+++ readTopicText( $web, $topic, $rev, $ignorePermissions ) -> $text
+
+Read topic text, including meta data
+   * =$web=                - Web name, e.g. ='Main'=, or empty
+   * =$topic=              - Topic name, e.g. ='MyTopic'=, or ="Main.MyTopic"=
+   * =$rev=                - Topic revision to read, optional. Specify the minor part of the revision, e.g. ="5"=, not ="1.5"=; the top revision is returned if omitted or empty.
+   * =$ignorePermissions=  - Set to ="1"= if checkAccessPermission() is already performed and OK; an oops URL is returned if user has no permission
+
+Return: =$text=                  Topic text with embedded meta data; an oops URL for calling redirectCgiQuery() is returned in case of an error
+
+*Deprecated: 6 Aug 2009. Use =readTopic= instead.
+This method returns meta-data embedded in the text. Plugins authors must be very careful to avoid damaging meta-data. Use readTopic instead, which is a lot safer and supports the full set of read options.
+
+=cut
+
+sub readTopicText {
+    my ( $web, $topic, $rev, $ignorePermissions ) = @_;
+    ASSERT($Foswiki::Plugins::SESSION) if DEBUG;
+
+    my $user;
+    $user = $Foswiki::Plugins::SESSION->{user}
+      unless defined($ignorePermissions);
+
+    my $topicObject =
+      Foswiki::Meta->load( $Foswiki::Plugins::SESSION, $web, $topic, $rev );
+
+    my $text;
+    if ( $ignorePermissions ||
+         $topicObject->haveAccess(
+             'VIEW', $Foswiki::Plugins::SESSION->{user} ) ) {
+        $text = $topicObject->getEmbeddedStoreForm();
+    }
+    else {
+        $text = getScriptUrl(
+            $web, $topic, 'oops',
+            template => 'oopsaccessdenied',
+            def      => 'topic_access',
+            param1   => 'VIEW',
+            param2   => $Foswiki::Meta::reason
+        );
+    }
+
+    return $text;
+}
+
+=begin TML
+
+---+++ saveTopicText( $web, $topic, $text, \%options ) -> $oopsUrl
+
+Save topic text, typically obtained by readTopicText(). Topic data usually includes meta data; the file attachment meta data is replaced by the meta data from the topic file if it exists.
+   * =$web=                - Web name, e.g. ='Main'=, or empty
+   * =$topic=              - Topic name, e.g. ='MyTopic'=, or ="Main.MyTopic"=
+   * =$text=               - Topic text to save, assumed to include meta data
+   * =$ignorePermissions=  - Set to ="1"= if checkAccessPermission() is already performed and OK
+   * =$dontNotify=         - Set to ="1"= if not to notify users of the change
+
+*Deprecated* 6 Aug 2009 - use saveTopic instead.
+=saveTopic= supports embedded meta-data in the saved text, and also
+supports the full set of save options.
+
+Return: =$oopsUrl=               Empty string if OK; the =$oopsUrl= for calling redirectCgiQuery() in case of error
+
+<verbatim>
+my $text = Foswiki::Func::readTopicText( $web, $topic );
+
+# check for oops URL in case of error:
+if( $text =~ /^http.*?\/oops/ ) {
+    Foswiki::Func::redirectCgiQuery( $query, $text );
+    return;
+}
+# do topic text manipulation like:
+$text =~ s/old/new/g;
+# do meta data manipulation like:
+$text =~ s/(META\:FIELD.*?name\=\"TopicClassification\".*?value\=\")[^\"]*/$1BugResolved/;
+$oopsUrl = Foswiki::Func::saveTopicText( $web, $topic, $text ); # save topic text
+</verbatim>
+
+=cut
+
+sub saveTopicText {
+    my ( $web, $topic, $text, $ignorePermissions, $dontNotify ) = @_;
+    ASSERT($Foswiki::Plugins::SESSION) if DEBUG;
+
+    my $session = $Foswiki::Plugins::SESSION;
+
+    # extract meta data and merge old attachment meta data
+    require Foswiki::Meta;
+    my $topicObject = Foswiki::Meta->new( $session, $web, $topic );
+    $topicObject->remove('FILEATTACHMENT');
+
+    my $oldMeta = Foswiki::Meta->load( $session, $web, $topic );
+    $topicObject->copyFrom( $oldMeta, 'FILEATTACHMENT' );
+
+    my $outcome = '';
+    unless ( $ignorePermissions || $topicObject->haveAccess('CHANGE') ) {
+        my @caller = caller();
+        return getScriptUrl(
+            $web, $topic, 'oops',
+            template => 'oopsattention',
+            def      => 'topic_access',
+            param1   => ( $caller[0] || 'unknown' )
+        );
+    }
+    $topicObject->text($text);
+
+    try {
+        $topicObject->save( minor => $dontNotify );
+    }
+    catch Error::Simple with {
+        $outcome = getScriptUrl(
+            $web, $topic, 'oops',
+            template => 'oopsattention',
+            def      => 'save_error',
+            param1   => shift->{-text}
+        );
+    };
+    return $outcome;
 }