Re: [evolution-patches] patch review for crash bug

[Jeffrey Stedfast <fejj ximian com>]

On Mon, 2003-12-01 at 16:22, Not Zed wrote:
> An even better (there is never a proper :) fix would be:
> 
> if (*s == '%' && isxdigit(s[1]) && isxdigit(s[2])) {
> *d++ = (XDIGIT (s[1]) << 4) + XDIGIT (s[2]);
> s += 2;
> } else
> *d++ = *s;

not quite... what if we have %%<xdigit><xdigit> ? :)

ah, but this isn't printf format... so I dunno. in that case, it should
probably just skip the first % anyway then since it'd be illegal I
gather? what do the url specs say?

> 
> The extra logic below is redundant.
> 
> As for the actual bug this is supposed to fix, the logic must be wrong
> elsewhere, it isn't encoding hte url correctly, which it should be, so
> this 'fix' isn't really required for that bug, all it is doing is
> hiding that bug.  (the fix is still required since we can get url's
> from external sources).

right, I agree.

Jeff

> 
> On Tue, 2003-12-02 at 04:05, Jeffrey Stedfast wrote: 
> > the proper fix would be to change:
> > 
> > if (*s == '%' && s[1] && s[2]) {
> > 
> > to
> > 
> > if (*s == '%') {
> >    if (isxdigit (s[1]) && isxdigit (s[2])) {
> >       *d++ = (XDIGIT (s[1]) << 4) + XDIGIT (s[2]);
> >       s += 2;
> >    } else {
> >       *d++ = *s;
> >       if (s[1] == '%')
> >         *d++ = *s++;
> >    }
> > } else
> >    *d++ = *s;
> > 
> > Jeff
> > 
> > On Mon, 2003-12-01 at 06:31, Calvin Liu wrote:
> > > Hi, there,
> > > 
> > > Here's a small patch to fix a crash bug.
> > > If you create vfolder like "xxx%%$$" it'll crash  on solaris or
> > > mis-displayed.
> > > Suppose we shouldn't decode for all "%xx" case, the "x" here should be
> > > {digital|a~z|A~z}.
> > > 
> > > Thanks.
> > > Calvin
> > 
> > _______________________________________________
> > Evolution-patches mailing list
> > Evolution-patches lists ximian com
> > http://lists.ximian.com/mailman/listinfo/evolution-patches