[pLog-svn] r6503 -plugins/branches/lifetype-1.2/editcomments/class/action

Jon Daley plogworld at jon.limedaley.com
Thu Jun 5 08:02:31 EDT 2008


 	I really don't care.  I don't know why you want to include 
negative ids (which is what you get with a case), and who knows what other 
values php might translate.
 	I am tired of you criticizing every single code checkin.  Feel 
free to do some real work.

On Thu, 5 Jun 2008, Mark Wu wrote:

> 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
>
> _______________________________________________
> pLog-svn mailing list
> pLog-svn at devel.lifetype.net
> http://limedaley.com/mailman/listinfo/plog-svn
>

-- 
Jon Daley
http://jon.limedaley.com
~~
Time is nature's way of keeping everything from happening at once.
-- Woody Allen


More information about the pLog-svn mailing list