[pLog-svn] r6503 -plugins/branches/lifetype-1.2/editcomments/class/action
Mark Wu
markplace at gmail.com
Thu Jun 5 22:56:02 EDT 2008
Myabe we should hold a meeting in IM with Oscar & Reto, see how we can solve
this issues ... And share loading to all of us evenly.
Mark
> -----Original Message-----
> From: Mark Wu [mailto:markplace at gmail.com]
> Sent: Friday, June 06, 2008 10:50 AM
> To: 'LifeType Developer List'
> Subject: RE: [pLog-svn] r6503
> -plugins/branches/lifetype-1.2/editcomments/class/action
>
> Actually, I have the same thoughts as you.
>
> I did consider left the project and fork a new project based
> on lifetype for several times... Coding is more fun than
> administrative job.
>
> You do the administrative job in English site, but I have to
> do all administrative jobs in Chinese sites without any
> reward( It is open source). It occupid most of my time ...
>
> Only one thing, I think it is lucky for me that I have
> several medorators can share some of my loadings in Chinese sites.
>
> About the framework, I do really have a lot of thoughts,
> includes, Ajax, ORM , Route and Validator ...
>
> Anyway, do you think we can improve the current situation?
> Any thoughts?
>
> 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: Friday, June 06, 2008 10:33 AM
> > To: LifeType SVN
> > Subject: Re: [pLog-svn] r6503
> > -plugins/branches/lifetype-1.2/editcomments/class/action
> >
> > The project hasn't been as fun as it was since Oscar
> left. I don't
> > really feel like I am working on a team, just me doing everything.
> > I have thought about leaving a number of times, and more about it
> > today.
> > It wouldn't really change anything for me, ie. I would fork the
> > project, and continue developing as I am, but I wouldn't
> answer stuff
> > on the forums, so I would have more time to code, and less time
> > discussing why someone else thinks their way is better, on
> something
> > that hardly matters, and their method isn't any better than the
> > current version.
> > I would have a whole lot less administrative work, and more fun
> > coding.
> > I am disappointed in the architecture, and how hard it is to
> > validate the inputs. It is very likely that whenever a new
> input is
> > added, it won't be validated appropriately. A number of
> our current
> > validators aren't very good, there are security holes all over the
> > place.
> > I guess it is because lifetype is small enough so the
> spammers haven't
> > taken that much notice of it.
> > Questioning commits is fine, and that is a good thing.
> > Lots of bugs have been fixed by others watching the commits.
> > In this case, your solution at best isn't any better than
> mine, and as
> > I have argued, is less clear about what values are accepted than a
> > simple regular expression that any php developer can understand
> > exactly how it works. Casting in most programming languages is the
> > wrong solution to almost every problem, and while php does a better
> > job than some languages, I wouldn't want to work on any
> code that uses
> > casts to do validation, that isn't what they are made for.
> > Probably more frustration than anger, I think.
> >
> >
> > On Fri, 6 Jun 2008, Mark Wu wrote:
> > > Hi Jon:
> > >
> > > It seems you get angry yesterday.
> > >
> > > If you feel unhappy about the discussion, I am sorry about that.
> > >
> > > But, you have to understand, it is not a good way to show
> > your anger
> > > in the mailing list.
> > >
> > > I think you remember you criticized or questioned my
> > commits or others
> > > commits all the time.
> > >
> > > And I have to explain every details for you to let you know
> > what I did
> > > in those commits. I just suppose you like this kind of
> discussions.
> > >
> > > If you think this kind of dicussion is not a good way to
> work in a
> > > team, maybe we should try to improve it, not just get angry.
> > >
> > > You know English is not my native language, so .. It may be not
> > > present my thoughts very well...
> > >
> > > If you feel bad with my wording or syntax, just let me know
> > directly.
> > >
> > > 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 8:03 PM
> > >> To: LifeType Developer List
> > >> Subject: Re: [pLog-svn] r6503
> > >> -plugins/branches/lifetype-1.2/editcomments/class/action
> > >>
> > >> 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
More information about the pLog-svn
mailing list