[pLog-svn] r6503 -plugins/branches/lifetype-1.2/editcomments/class/action
Mark Wu
markplace at gmail.com
Thu Jun 5 08:00:11 EDT 2008
I think I know your problem now, but filteingr the value is not a proper
way, here comes some thoughts:
1. if _op!= updateComments, we should use the comment list as error view
2. if _op== updateComments, we should check the commentId exist or not, if
yes, use the editCommentView, if not use the comment list view.
A better security check is we also check the blog id is in comment is equal
to the blog id in blogInfo or not..
Mark
====================
function AdminUpdateCommentAction($actionInfo, $request){
$this->AdminAction($actionInfo, $request);
$this->requirePermission( "update_comment" );
$this->_op = $actionInfo->getActionParamValue();
if($this->_request->getValue("cancel")){
$this->_op = "cancel";
}
// articleId is needed, even on a cancel operation
$this->registerFieldValidator("articleId", new IntegerValidator());
// should use a filter instead
$this->_articleId = $this->_request->getValue( "articleId" );
if($this->_op == "updateComment"){
$this->registerFieldValidator("commentId", new
IntegerValidator());
$this->registerFieldValidator("commentText", new
StringValidator(true));
$this->registerFieldValidator("authorName", new
StringValidator(false));
$this->registerFieldValidator("commentStatus", new
IntegerValidator());
$this->registerFieldValidator("commentTopic", new
StringValidator(false), true);
$this->registerFieldValidator("authorEmail", new
EmailValidator(), true);
$this->registerFieldValidator("authorUrl", new
HttpUrlValidator(), true);
$this->registerFieldValidator("commentDateTime", new
StringValidator(false));
$this->registerFieldValidator("commentIp", new
StringValidator(false));
// should use a filter instead
$this->_commentId = (int)
$this->_request->getValue("commentId");
$comments = new ArticleComments();
if( $comments->getComment( $this->_commentId ) )
{
$view = new AdminEditCommentView($this->_blogInfo,
$this->_commentId);
$view->setErrorMessage(
$this->_locale->tr("pluginEditCommentsInvalidData"));
$this->setValidationErrorView( $view );
$this->_fetchFields();
} else {
$view = new
AdminArticleCommentsListView($this->_blogInfo, array("article" => 0));
$view->setErrorMessage(
$this->_locale->tr("error_articleid_should_be_intege"));
$this->setValidationErrorView( $view );
}
} else {
$view = new AdminArticleCommentsListView($this->_blogInfo,
array("article" => 0));
$view->setErrorMessage(
$this->_locale->tr("error_articleid_should_be_intege"));
$this->setValidationErrorView( $view );
}
> -----Original Message-----
> From: plog-svn-bounces at devel.lifetype.net
> [mailto:plog-svn-bounces at devel.lifetype.net] On Behalf Of Jon Daley
> Sent: Thursday, June 05, 2008 7:30 PM
> To: LifeType Developer List
> Subject: Re: [pLog-svn] r6503
> -plugins/branches/lifetype-1.2/editcomments/class/action
>
> Please try it and see what happens, I don't know how to
> explain it to you.
>
> On Thu, 5 Jun 2008, Mark Wu wrote:
>
> > I don't think type casting is ambiguous, anyway, it is
> personal choice.
> >
> > But, I still don't't get it... It is the code from
> > adminupdatecommentaction.class.php
> >
> > // articleId is needed, even on a cancel operation
> > $this->registerFieldValidator("articleId", new
> IntegerValidator());
> > // should use a filter instead
> > $this->_articleId = preg_replace("/[^0-9]/", "",
> > $this->_request->getValue( "articleId" ));
> >
> > If the the articleId is not an integer, why you need to filter it??
> > Our registerFiledValidator will reject it..
> >
> > You never have a chance to use it ....
> >
> > $this->_commentId has the same issues, if it reject by
> > registerFiledValidator, you never have a chance to use it...
> >
> > Mark
> >
> >
> >> -----Original Message-----
> >> From: plog-svn-bounces at devel.lifetype.net
> >> [mailto:plog-svn-bounces at devel.lifetype.net] On Behalf Of Jon Daley
> >> Sent: Thursday, June 05, 2008 7:10 PM
> >> To: LifeType Developer List
> >> Subject: Re: [pLog-svn] r6503
> >> -plugins/branches/lifetype-1.2/editcomments/class/action
> >>
> >> I won't ever use a cast to do that. preg_replace is
> less ambiguous
> >> about what characters are allowed.
> >>
> >> On Thu, 5 Jun 2008, Mark Wu wrote:
> >>
> >>> If it is a constructer, then use casting (int) articleId
> >> will be better.
> >>>
> >>> Mark
> >>>
> >>>> -----Original Message-----
> >>>> From: plog-svn-bounces at devel.lifetype.net
> >>>> [mailto:plog-svn-bounces at devel.lifetype.net] On Behalf
> Of Jon Daley
> >>>> Sent: Thursday, June 05, 2008 7:00 PM
> >>>> To: LifeType Developer List
> >>>> Subject: Re: [pLog-svn] r6503
> >>>> -plugins/branches/lifetype-1.2/editcomments/class/action
> >>>>
> >>>> How do you suggest we reject it in the constructor?
> >>>>
> >>>> On Thu, 5 Jun 2008, Mark Wu wrote:
> >>>>
> >>>>> Why we need to filter it??
> >>>>>
> >>>>> If articleId is not integer, we should reject the request
> >>>> instead of
> >>>>> filter it....
> >>>>>
> >>>>> Mark
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: plog-svn-bounces at devel.lifetype.net
> >>>>>> [mailto:plog-svn-bounces at devel.lifetype.net] On Behalf Of
> >>>>>> jondaley at devel.lifetype.net
> >>>>>> Sent: Thursday, June 05, 2008 6:52 PM
> >>>>>> To: plog-svn at devel.lifetype.net
> >>>>>> Subject: [pLog-svn] r6503
> >>>>>> -plugins/branches/lifetype-1.2/editcomments/class/action
> >>>>>>
> >>>>>> Author: jondaley
> >>>>>> Date: 2008-06-05 06:51:52 -0400 (Thu, 05 Jun 2008) New
> >>>> Revision: 6503
> >>>>>>
> >>>>>> Modified:
> >>>>>>
> >>>>>> plugins/branches/lifetype-1.2/editcomments/class/action/adminu
> >>>>>> pdatecommentaction.class.php
> >>>>>> Log:
> >>>>>> we need to manually filter the ids since we are grabbing
> >>>> them in the
> >>>>>> constructor. a 'real' Filter() would be better, but
> >>>>>> 1.2 doesn't have very many filters available. We'll need
> >>>> to add them
> >>>>>> in 2.0
> >>>>>>
> >>>>>> Modified:
> >>>>>> plugins/branches/lifetype-1.2/editcomments/class/action/adminu
> >>>>>> pdatecommentaction.class.php
> >>>>>>
> >> ===================================================================
> >>>>>> ---
> >>>>>> plugins/branches/lifetype-1.2/editcomments/class/action/adminu
> >>>>>> pdatecommentaction.class.php 2008-06-05 10:49:33 UTC
> >>>> (rev 6502)
> >>>>>> +++
> >>>>>> plugins/branches/lifetype-1.2/editcomments/class/action/adminu
> >>>>>> pdatecommentaction.class.php 2008-06-05 10:51:52 UTC
> >>>> (rev 6503)
> >>>>>> @@ -36,7 +36,8 @@
> >>>>>>
> >>>>>> // articleId is needed, even on a cancel operation
> >>>>>> $this->registerFieldValidator("articleId", new
> >>>>>> IntegerValidator());
> >>>>>> - $this->_articleId = $this->_request->getValue(
> >>>> "articleId" );
> >>>>>> + // should use a filter instead
> >>>>>> + $this->_articleId = preg_replace("/[^0-9]/", "",
> >>>>>> + $this->_request->getValue( "articleId" ));
> >>>>>>
> >>>>>> if($this->_op == "updateComment"){
> >>>>>> $this->registerFieldValidator("commentId", new
> >>>>>> IntegerValidator()); @@ -48,8 +49,9 @@
> >>>>>> $this->registerFieldValidator("authorUrl", new
> >>>>>> HttpUrlValidator(), true);
> >>>>>> $this->registerFieldValidator("commentDateTime",
> >>>>>> new StringValidator(false));
> >>>>>> $this->registerFieldValidator("commentIp", new
> >>>>>> StringValidator(false));
> >>>>>> -
> >>>>>> - $this->_commentId =
> >>>>>> $this->_request->getValue("commentId");
> >>>>>> +
> >>>>>> + // should use a filter instead
> >>>>>> + $this->_commentId = preg_replace("/[^0-9]/", "",
> >>>>>> + $this->_request->getValue("commentId"));
> >>>>>> $view = new
> >>>>>> AdminEditCommentView($this->_blogInfo, $this->_commentId);
> >>>>>>
> >>>>>> $view->setErrorMessage(
> >>>>>> $this->_locale->tr("pluginEditCommentsInvalidData"));
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> pLog-svn mailing list
> >>>>>> pLog-svn at devel.lifetype.net
> >>>>>> http://limedaley.com/mailman/listinfo/plog-svn
> >>>>>
> >>>>> _______________________________________________
> >>>>> pLog-svn mailing list
> >>>>> pLog-svn at devel.lifetype.net
> >>>>> http://limedaley.com/mailman/listinfo/plog-svn
> >>>>>
> >>>>
> >>>> --
> >>>> Jon Daley
> >>>> http://jon.limedaley.com
> >>>> ~~
> >>>> If everything is coming your way then you're in the wrong lane.
> >>>> -- Anonymous
> >>>> _______________________________________________
> >>>> pLog-svn mailing list
> >>>> pLog-svn at devel.lifetype.net
> >>>> http://limedaley.com/mailman/listinfo/plog-svn
> >>>
> >>> _______________________________________________
> >>> pLog-svn mailing list
> >>> pLog-svn at devel.lifetype.net
> >>> http://limedaley.com/mailman/listinfo/plog-svn
> >>>
> >>
> >> --
> >> Jon Daley
> >> http://jon.limedaley.com
> >> ~~
> >> No matter where you go,
> >> there you are.
> >> -- Buckaroo Bonzai
> >> _______________________________________________
> >> pLog-svn mailing list
> >> pLog-svn at devel.lifetype.net
> >> http://limedaley.com/mailman/listinfo/plog-svn
> >
> > _______________________________________________
> > pLog-svn mailing list
> > pLog-svn at devel.lifetype.net
> > http://limedaley.com/mailman/listinfo/plog-svn
> >
>
> --
> Jon Daley
> http://jon.limedaley.com
> ~~
> A professor is one who can speak on any subject: for
> precisely fifty minutes.
> -- Norbert Weiner
> _______________________________________________
> pLog-svn mailing list
> pLog-svn at devel.lifetype.net
> http://limedaley.com/mailman/listinfo/plog-svn
More information about the pLog-svn
mailing list