Good point, and I agree, allowing the user to use their own cursor wrappers 
is probably the best solution. At least, it would allow us to avoid the 
monkeypatching we're having to use here. From there, this particular 
wrapping could just turn into a django-snippet. I'll try to work on 
something oriented towards that instead.  It may take a 
while--priorities--but I'll post back here when I come up with something. 
Thanks!

On Thursday, October 25, 2012 10:06:51 AM UTC-5, Anssi Kääriäinen wrote:
>
> On 25 loka, 17:52, Marty Woodlee <marty.wood...@gmail.com> wrote: 
> > Yeah, I don't think we'd try to do anything like that in our actual 
> Django 
> > patch... the THREADLOCALS thing has worked well for us but I realize 
> that's 
> > a pretty big shift to impose on others. If anything I think the patch 
> might 
> > simply modify the existing CursorDebugWrapper to add the traceback-based 
> > comment based on DEBUG being True (and/or some other setting). 
> > 
> > Also, we discovered a small bug in the above for cases where manual 
> queries 
> > were being sent with semicolons already on them. MySQL was seeing it as 
> two 
> > queries in the same cursor execution and raising a ProgrammingError. 
> Just 
> > in case someone stumbles across this while Googling and wants to use 
> this 
> > solution, here's the diff vs. the above code that fixes it: 
> > 
> > -        sql += origin_comment + ' */' 
> > +        sql = sql.rstrip(' ;') 
> > +        sql = ''.join([sql, origin_comment, ' */']) 
>
> Some other possible problems: 
>   - In PostgreSQL comments nest (this is the SQL standard behaviour, I 
> don't know if other DBs implement this or the non-nesting comments 
> approach). 
>   - If the query ends in line comment (one starting with --) and an 
> attacker manages to supply in a newline then there is potential for 
> SQL injection. This should not be possible if using the uri or single 
> line of the stack trace but worth checking still in the code. 
>
> We have currently two different CursorWrappers, debug and default one. 
> Having a way to supply different CursorWrappers would allow users to 
> do anything they want, maybe rewriting some queries to use stored 
> procedures or whatever... We could also have some often needed 
> cursorwrappers in core. 
>
> I don't think it is a good idea to implement something that will allow 
> annotating the source of the query in SQL comment but nothing else. 
>
>  - Anssi 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/jjQ6CIrA0nEJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Reply via email to