Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Dancer-Plugin-Database CPAN distribution.

Report information
The Basics
Id: 66789
Status: resolved
Priority: 0/
Queue: Dancer-Plugin-Database

People
Owner: Nobody in particular
Requestors: ms [...] 2scale.net
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Cosmetic "bug" in Dancer::Plugin::Database::Handler
Date: Tue, 22 Mar 2011 22:18:09 +0100
To: David Precious via RT <bug-Dancer-Plugin-Database [...] rt.cpan.org>
From: Michael Stiller <ms [...] 2scale.net>
Hi, consider e.g. using database->quick_update with blob data. You will end up with a (huge) debug log entry consisting of unreadable characters. (Which will also drive your terminal mad) Maybe change the debug log line to something like this: --- Handle.pm.orig 2011-03-22 21:53:15.000000000 +0100 +++ Handle.pm 2011-03-22 22:06:45.000000000 +0100 @@ -158,7 +158,8 @@ } Dancer::Logger::debug( - "Executing $type query $sql with params " . join ',', @bind_params + "Executing $type query $sql with params " . join ',', + map { $_ =~ /^[[:ascii:]]+$/ ? $_ : "'NO-ASCII'" } @bind_params ); Cheers, Michael -- 2scale GmbH, Karlstr. 88, 40210 Düsseldorf Amtsgericht: Düsseldorf HRB 50718 Geschäftsführer: Georg von Zezschwitz, Dirk Vleugels USt-IdNr.: DE 210936505
On 2011-03-22 17:18:21, ms@2scale.net wrote: Show quoted text
> consider e.g. using database->quick_update with blob data. > You will end up with a (huge) debug log entry consisting of unreadable > characters. (Which will also drive your terminal mad)
That's a very good point - that's not very desirable! I'll get a new version out soon incorporating the change you suggested to avoid this problem. Thanks for your feedback!
On 2011-03-22 18:49:20, BIGPRESH wrote: Show quoted text
> On 2011-03-22 17:18:21, ms@2scale.net wrote:
> > consider e.g. using database->quick_update with blob data. > > You will end up with a (huge) debug log entry consisting of unreadable > > characters. (Which will also drive your terminal mad)
> > That's a very good point - that's not very desirable! > > I'll get a new version out soon incorporating the change you suggested > to avoid this problem.
I've just pushed commit 6f9be5b which does this, and also truncates long values (for instance, let's say it was updating a TEXT column with a blog post - logging the whole thing is probably excessively noisy and unhelpful :) However, now I think about this, I think it might be sensible to disable this by default and not even call Dancer::Logger::debug() with the message unless a `log_queries` option is set in the plugin config, for two reasons: * it would avoid the overhead of calling Dancer::Logger::debug for every query (which is wasted effort if the app's log level is set to not log at level debug anyway) * it could be a potential security issue for people who didn't realise that, whilst in debug mode, queries they perform are logged. If they have to explictly turn on query logging, they should stop to think what that means (and the documentation for the option can point out that this could mean potentially-sensitive database contents also appearing in the log file/other log destination, if running the app with a debug log level). I'll have a further think about this.
On 2011-03-23 00:13:35, BIGPRESH wrote: Show quoted text
> However, now I think about this, I think it might be sensible to disable > this by default and not even call Dancer::Logger::debug() with the > message unless a `log_queries` option is set in the plugin config, for > two reasons: [...]
I had a think about it and decided that only logging the generated queries if a log_queries setting was enabled was the safest course of action, to avoid people accidentally logging potentially sensitive data if they left debug logging on in a production environment. Therefore, I've just released version 1.23, which only logs queries generated by quick_select(), quick_insert() etc if the log_queries setting is enabled. Thanks for making me think more about this feature :)