[pLog-svn] r6503 -plugins/branches/lifetype-1.2/editcomments/class/action
Mark Wu
markplace at gmail.com
Thu Jun 5 22:50:03 EDT 2008
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