Fwd: Re: SafeString usage

[Tim Almdal]

----- Original Message -----
From: Andy Staudacher <andy.st@...>
Date: Thursday, September 17, 2009 1:28 pm
Subject: Re: SafeString usage
To: Tim Almdal <tnalmdal@...>
Cc: Bharat Mediratta <bharat@...>

> +CC gallery-devel? Could be useful for others as well.
>
> On Thu, Sep 17, 2009 at 1:15 PM, Tim Almdal
> <tnalmdal@...> wrote:
>
> > Could you please look at this commit:
> >
> >
> http://github.com/gallery/gallery3/commit/48326ad01708fcfa020283e2ad8b2cae4ede1600>
> > if I don't add the ->__toString() call then what goes in the
> error message
> > and displayed on the screen is [object Object]. 
> $error_message is included
> > in a json response a little further down in the code.
> >
> > Is this a problem with my usage, SafeString or the php json enoding
> > routines?
> >
>
> General remarks:
>
> 1. Style: For SafeString, prefer $var->for_html() or (string)
> $var over
> $var->__toString()
> 2. Style: Generally, prefer (string) over __toString().
> __toString() is a
> PHP internal magic method, don't invoke it directly.
> 3. In most cases, $var will do, no explicit casting to string
> necessary.
> @SafeString -> JSON replies:
> Yes, that's a case where you'll want to cast to string since
> json_encode()will treat it as object otherwise.
>
>  - Andy
>
>
> > Here's the relevant discussion that Bharat & I had on irc:
> > 12:56] <+bharat>
> >
> http://github.com/gallery/gallery3/commit/48326ad01708fcfa020283e2ad8b2cae4ede1600> [12:56] <+bharat> do you need ->__toString() there?
> > [12:56] <+talmdal> ohoh
> > [12:57] <+talmdal> yep, otherwise i display [object Object]
> as the error
> > message
> > [12:58] <+bharat> huh.  that's odd
> > [12:58] <+talmdal> not really
> > [12:58] <+bharat> can you ping valiant about that? 
> doesn't make sense to
> > me
> > [12:58] <+bharat> SafeString shouldn't require us to do that
> > [12:59] <+bharat> ie: we should not settle for that limitation
> > [12:59] <+bharat> (how come you're not yelling about having
> to put that in
> > the code?)
> > [12:59] <+talmdal> because we are not rendering a view, but
> passing> $error_msg which then gets formatted as a json object
> to response
> > [13:00] <+bharat> but at some point there's an implcit
> string conversion
> > that should happen
> > [13:00] <+bharat> that pattern you have there is wrong
> because it means
> > that we're losing the SafeString-ness of it right away
> > [13:00] <+bharat> so downstream, we no longer know that
> it's safe
> > [13:00] <+bharat> the point is to only get rid of the
> SafeString at the
> > very end
> > [13:01] <+bharat> gtg, mtg
> > [13:01] <+talmdal> i could cast it to string when the
> output json message
> > is formatted
> > [13:01] <+bharat> we shouldn't have to do that either
> > [13:02] <+bharat> if we do, then we screwed this up
> >
> >
> >
> >
> > Website: http://www.timalmdal.com
> >
> >
>

Website: http://www.timalmdal.com


------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]

[Andy Staudacher-3]

On Thu, Sep 17, 2009 at 1:31 PM, Tim Almdal <tnalmdal@...> wrote:

----- Original Message -----
From: Andy Staudacher <[e-mail]>
Date: Thursday, September 17, 2009 1:28 pm
Subject: Re: SafeString usage
To: Tim Almdal <tnalmdal@...>
Cc: Bharat Mediratta <bharat@...>

> +CC gallery-devel? Could be useful for others as well.
>
> On Thu, Sep 17, 2009 at 1:15 PM, Tim Almdal
> <tnalmdal@...> wrote:
>
> > Could you please look at this commit:
> >
> >
> http://github.com/gallery/gallery3/commit/48326ad01708fcfa020283e2ad8b2cae4ede1600>
> > if I don't add the ->__toString() call then what goes in the
> error message
> > and displayed on the screen is [object Object]. 
> $error_message is included
> > in a json response a little further down in the code.
> >
> > Is this a problem with my usage, SafeString or the php json enoding
> > routines?
> >
>
> General remarks:
>
> 1. Style: For SafeString, prefer $var->for_html() or (string)
> $var over
> $var->__toString()
> 2. Style: Generally, prefer (string) over __toString().
> __toString() is a
> PHP internal magic method, don't invoke it directly.
> 3. In most cases, $var will do, no explicit casting to string
> necessary.
> @SafeString -> JSON replies:
> Yes, that's a case where you'll want to cast to string since
> json_encode()will treat it as object otherwise.

Actually, for_html() returns the object again. So you'll need to use (string) for this specific case.

And regarding Bharat's remark: No, calling __toString() or casting to (string) won't defeat SafeString's security mechanisms. The escaping happens in __toString().

I don't consider this to be a red flag for the SafeString / t() API. It's very rare that we have to cast to string explicitly. In most use cases, The cast happens implicitly.

 - Andy

 - Andy

 

>
>  - Andy
>
>
> > Here's the relevant discussion that Bharat & I had on irc:
> > 12:56] <+bharat>
> >
> http://github.com/gallery/gallery3/commit/48326ad01708fcfa020283e2ad8b2cae4ede1600> [12:56] <+bharat> do you need ->__toString() there?
> > [12:56] <+talmdal> ohoh
> > [12:57] <+talmdal> yep, otherwise i display [object Object]
> as the error
> > message
> > [12:58] <+bharat> huh.  that's odd
> > [12:58] <+talmdal> not really
> > [12:58] <+bharat> can you ping valiant about that? 
> doesn't make sense to
> > me
> > [12:58] <+bharat> SafeString shouldn't require us to do that
> > [12:59] <+bharat> ie: we should not settle for that limitation
> > [12:59] <+bharat> (how come you're not yelling about having
> to put that in
> > the code?)
> > [12:59] <+talmdal> because we are not rendering a view, but
> passing> $error_msg which then gets formatted as a json object
> to response
> > [13:00] <+bharat> but at some point there's an implcit
> string conversion
> > that should happen
> > [13:00] <+bharat> that pattern you have there is wrong
> because it means
> > that we're losing the SafeString-ness of it right away
> > [13:00] <+bharat> so downstream, we no longer know that
> it's safe
> > [13:00] <+bharat> the point is to only get rid of the
> SafeString at the
> > very end
> > [13:01] <+bharat> gtg, mtg
> > [13:01] <+talmdal> i could cast it to string when the
> output json message
> > is formatted
> > [13:01] <+bharat> we shouldn't have to do that either
> > [13:02] <+bharat> if we do, then we screwed this up
> >
> >
> >
> >
> > Website: http://www.timalmdal.com
> >
> >
>

Website: http://www.timalmdal.com

------------------------------------------------------------------------------
Come build with us! The BlackBerry&reg; Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9&#45;12, 2009. Register now&#33;
http://p.sf.net/sfu/devconf
__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]