Re: [web2py] Re: Recent change to string escaping breaks dynamic javascript

[Michael Ellis]

My bad.  It does work.  In my earlier attempt to use it I forgot that a for
loop variable isn't an object reference when looping over a list of strings.
 So the value of schartoptions wasn't being altered at all, just the loop
variable.

I'm still in favor of an alternate operator, though.  As it is now, I've
moved all my chart options inside the script tag e.g.

    var schartoptions = {
        type: 'bar',
        barColor: 'green',
        chartRangeMin: '{{=chartmin}}',
        chartRangeMax: '{{=chartmax}}'
    };

and substitute only the things that have to be determined  at run time
rather than clutter up my expressions by wrapping them in XML().

I'd much rather code in python than js, but I'm beginning to feel that using
more js and less python in views makes a lot of sense.


Thanks,
Mike


On Sat, Jul 24, 2010 at 8:40 AM, mdipierro <mdipie...@cs.depaul.edu> wrote:

> I am confused:
> Does this now work?
>
> {{
> schartoptions = """{
>    type: 'bar',
>    barColor: 'green',
>    chartRangeMin: '%d',
>    chartRangeMax: '%d'
>    }
>    """%(chartmin,chartmax)
>
> }}
>
> and later on I use the variables within a script tag, e.g.
>
> <script type="text/javascript">
>    /* <![CDATA[ */
>    $("#{{=ks+kc}}").sparkline(data.wsc.{{=ks}}.{{=kc}},
> {{=XML(schartoptions)}}
> </script>
>
> If not, what are chartmin and chartmax, are they themselves helpers?
>
> On Jul 24, 7:28 am, Michael Ellis <michael.f.el...@gmail.com> wrote:
> > Massimo, I'm not following you.  I tried using XML (see earlier post) and
> it
> > had no effect.  Does it only work if applied immediately before the =
> > operator?
> >
> > Also, I think ":=" or something similar is much cleaner than wrapping
> > everything in a function call.
> >
> > Cheers,
> > Mike
> >
> > On Sat, Jul 24, 2010 at 8:19 AM, mdipierro <mdipie...@cs.depaul.edu>
> wrote:
> > > This
> >
> > > {{:=never_escaped}}
> >
> > > would be the same as
> >
> > > {{=XML(ever_escaped)}}
> >
> > > so why introduce new syntax?
> >
> > > On Jul 24, 7:14 am, Michael Ellis <michael.f.el...@gmail.com> wrote:
> > > > I could happily live with a solution that adds a 'no escape' operator
> to
> > > the
> > > > template language, e.g.
> >
> > > > {{:=never_escaped}}
> >
> > > > vs
> >
> > > > {{=always_escaped}}
> >
> > > > 1. Backward compatible,
> >
> > > > 2. Safe by default,
> >
> > > > 3. Allows designer to decide what's safe and what isn't,
> >
> > > > 4. Seems like  an easier fix than trying to make the rendering code
> smart
> > > > enough to always distinguish js from html strings.
> >
> > > > Just a thought,
> > > > Mike
> >
> > > > On Sat, Jul 24, 2010 at 4:02 AM, mdipierro <mdipie...@cs.depaul.edu>
> > > wrote:
> > > > > Thadeus,
> >
> > > > > This was a security fix. We had a a security review and this was
> > > > > determined to be a weakness. The code by Mike Ellis broke not
> because
> > > > > of the fix but because it incorrectly implicitly assumed that  the
> > > > > strings were HTML/XML and therefore needed escaping when, in
> reality,
> > > > > they were JS strings.
> >
> > > > > If we had a review board, would you have opposed to this change?
> >
> > > > > Massimo
> >
> > > > > On Jul 23, 5:28 pm, Thadeus Burgess <thade...@thadeusb.com> wrote:
> > > > > > I also agree that this is a break in backwards compatibility. It
> is
> > > also
> > > > > a
> > > > > > change that was never considered for longer than 15 minutes
> before
> > > the
> > > > > > decision to make the change was implemented.
> >
> > > > > > I really wish we would put certain things such as this under a
> review
> > > > > board
> > > > > > so they don't get into web2py and break things!
> >
> > > > > > --
> > > > > > Thadeus
> >
> > > > > > On Fri, Jul 23, 2010 at 2:33 PM, MikeEllis <
> > > michael.f.el...@gmail.com
> > > > > >wrote:
> >
> > > > > > > Typo: 2 sentence in prior message should read
> >
> > > > > > > " ... after XML() supplies the unescaped string."
> >
> > > > > > > On Jul 23, 3:28 pm, Michael Ellis <michael.f.el...@gmail.com>
> > > wrote:
> > > > > > > > Urgh!  FWIW, putting XML() around the strings doesn't seem to
> > > work.
> > > > > > >  Looks
> > > > > > > > like the escaping is applied after XML() supplies the
> unquoted
> > > > > string.
> >
> > > > > > > > I tried
> > > > > > > > {{
> > > > > > > > for optstring in (schartoptions, countpieoptions,
> cchartoptions):
> > > > > > > >     optstring = XML(optstring)
> > > > > > > >     debug("opstring=%s"%optstring)
> > > > > > > >     pass}}
> >
> > > > > > > > after assigning the strings and before they are used in
> inside
> > > the
> > > > > > > <script>
> > > > > > > > tags.
> >
> > > > > > > > The debug() calls show the strings with the single quotes
> > > unescaped,
> > > > > but
> > > > > > > > they still end up being escaped in what gets sent to browser.
> >
> > > > > > > > On Fri, Jul 23, 2010 at 2:16 PM, Michael Ellis <
> > > > > > > michael.f.el...@gmail.com>wrote:
> >
> > > > > > > > > Thanks, Nathan. That's certainly a possibility.  It's just
> that
> > > I'm
> > > > > not
> > > > > > > > > sure what security issue this change actually fixes.  There
> are
> > > no
> > > > > > > > > user-supplied strings in what I'm using to generate the
> jQuery
> > > > > calls.
> > > > > > >  If
> > > > > > > > > that were the case, then yes it would definitely be my
> > > > > responsibility
> > > > > > > to
> > > > > > > > > properly sanitize it.
> >
> > > > > > > > > Have to say this feels like a loss of  backward
> compatibility
> > > to
> > > > > me.
> > > > > > >  I've
> > > > > > > > > got a fair amount of code in this app that uses that
> technique;
> > > > > it's
> > > > > > > already
> > > > > > > > > inherently messy because of the indirection involved in
> code
> > > > > > > generation.
> > > > > > > > >  Wrapping it all in XML calls just adds to the mess.  Hope
> > > there's
> > > > > a
> > > > > > > way to
> > > > > > > > > refine the security fix so that it's confined to the areas
> that
> > > > > matter.
> >
> > > > > > > > > Cheers,
> > > > > > > > > Mike
> >
> > > > > > > > > On Fri, Jul 23, 2010 at 1:56 PM, mr.freeze <
> > > nat...@freezable.com>
> > > > > > > wrote:
> >
> > > > > > > > >> It was probably introduced as a security fix. You can do:
> > > > > > > > >> {{
> > > > > > > > >> schartoptions = XML("""{
> > > > > > > > >>     type: 'bar',
> > > > > > > > >>    barColor: 'green',
> > > > > > > > >>    chartRangeMin: '%d',
> > > > > > > > >>    chartRangeMax: '%d'
> > > > > > > > >>    }
> > > > > > > > >>    """%(chartmin,chartmax))
> > > > > > > > >> }}
> >
> > > > > > > > >> and it won't be escaped.
> >
> > > > > > > > >> On Jul 23, 12:39 pm, Michael Ellis <
> michael.f.el...@gmail.com
> >
> > > > > wrote:
> > > > > > > > >> > I've got an app with views that generate jQuery code on
> the
> > > fly.
> > > > > > >  This
> > > > > > > > >> was
> > > > > > > > >> > all working fine until recently, i.e. sometime after
> 1.92.
> > >  With
> > > > > > > more
> > > > > > > > >> recent
> > > > > > > > >> > builds, single and double quotes in strings are now
> escaped
> > > and
> > > > > it
> > > > > > > > >> breaks
> > > > > > > > >> > the javascript.   Here's an example
> >
> > > > > > > > >> > The view has (with much snipped out):
> >
> > > > > > > > >> > {{
> > > > > > > > >> > schartoptions = """{
> > > > > > > > >> >     type: 'bar',
> > > > > > > > >> >     barColor: 'green',
> > > > > > > > >> >     chartRangeMin: '%d',
> > > > > > > > >> >     chartRangeMax: '%d'
> > > > > > > > >> >     }
> > > > > > > > >> >     """%(chartmin,chartmax)
> >
> > > > > > > > >> > }}
> >
> > > > > > > > >> > and later on I use the variables within a script tag,
> e.g.
> >
> > > > > > > > >> > <script type="text/javascript">
> > > > > > > > >> >     /* <![CDATA[ */
> > > > > > > > >> >     $("#{{=ks+kc}}").sparkline(data.wsc.{{=ks}}.{{=kc}},
> > > > > > > > >> {{=schartoptions}}
> > > > > > > > >> > </script>
> >
> > > > > > > > >> > With an earlier web2py, it produces the desired result,
> >
> > > > > > > > >> > $("#solution0").sparkline(data.s.solution0, {
> > > > > > > > >> >     type: 'bar',
> > > > > > > > >> >     barColor: 'green',
> > > > > > > > >> >     chartRangeMin: '0',
> > > > > > > > >> >     chartRangeMax: '1'
> > > > > > > > >> >     }
> > > > > > > > >> >     );
> >
> > > > > > > > >> > but now (at tip) I get
> >
> > > > > > > > >> > $("#solution0").sparkline(data.s.solution0, {
> > > > > > > > >> >     type: &#x27;bar&#x27;,
> > > > > > > > >> >     barColor: &#x27;green&#x27;,
> > > > > > > > >> >     chartRangeMin: &#x27;0&#x27;,
> > > > > > > > >> >     chartRangeMax: &#x27;1&#x27;
> > > > > > > > >> >     }
> > > > > > > > >> > );
> >
> > > > > > > > >> > Was this change intentional?  If so, what's the
> recommended
> > > > > > > workaround?
> >
> > > > > > > > >> > Thanks,
> > > > > > > > >> > Mike
>