Re: [Catalyst] Re: Supressing passwords in debug messages
On 14 Nov 2009, at 22:03, Brian Phillips wrote: I don't recall any remaining changes that have been suggested that weren't implemented. t0m, I'm guessing that branch is pretty stale by now. Can we rebase or merge the current mainline to make sure it still works with the current state of Catalyst? I've merged current Catalyst trunk to the branch for you just now. It was a clean merge, but I haven't run the tests or anything, so ymmv :) Do you have any further suggestions on the branch in its current form? I've given myself a TODO note to look at the code again now that it is up to date. If you could also have a look over the branch and check you're happy and it still works then that'd be good. Should I just submit it to the mailing list as you originally suggested? Yeah, do something to avoid it getting forgotten again - yell on the lists, or poke people on irc or whatever :) Cheers t0m ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
On 13 Nov 2009, at 21:03, Geoff Flarity wrote: Bump. Anyone know the status of this feature? Even if it was available only as plugin it was would be incredibly useful. http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Runtime/5.80/branches/param_filtering I'm struggling to remember where this got to.. The patch looks ok, but not quite finished - IIRC I sent some feedback after an initial review and it didn't go any further than that :( Cheers t0m ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
Bump. Anyone know the status of this feature? Even if it was available only as plugin it was would be incredibly useful. Thanks, GF On Wed, Jul 1, 2009 at 11:26 AM, Brian Phillips bpphillips...@gmail.combpphillips%2...@gmail.com wrote: OK, I'm seriously ruining my first attempt at submitting a patch. :-) I changed the capitalization of a config key without updating the unit tests so blush here's another patch file. Apologies for the spam. I think this is the last one I'll need to post on this issue ... hopefully ... :-) On Wed, Jul 1, 2009 at 11:16 AM, Brian Phillips bpphillips...@gmail.combpphillips%2...@gmail.com wrote: Sorry for the repost but I realized I hadn't updated to svn HEAD before making the patch file. In case that matters, the attached should apply cleanly against r10759. Thanks! On Wed, Jul 1, 2009 at 11:03 AM, Brian Phillips bpphillips...@gmail.combpphillips%2...@gmail.com wrote: I've taken a stab at implementing this as I recently was wanting this functionality. See attached for the patch (including docs and unit tests). Feedback welcome. On Fri, Jan 30, 2009 at 5:20 PM, Byron Young byron.yo...@riverbed.comwrote: Tomas Doran wrote on 2009-01-29: On 29 Jan 2009, at 18:53, Byron Young wrote: Hi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff. No, reposting if things get dropped on the floor good :) If you have time, then arriving on #catalyst-dev and making noise also gets stuff done. This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): Having been discussed in #catalyst-dev, we think that the patch could be made both more generic, and more elegant. The key thing is to split the table drawing, and the data filtering into separate methods (maybe filter_debug_data?). This would then allow you to filter per-type, and support things such as redact_parameters (all), redact_body_parameters, redact_query_parameters, and even potentially to add support for filtering things like the URI (I can see use-cases where that'd be significant - e.g. not wanting to log session IDs which are in URIs).. Have a look at the way the debug screen stuff works (in Catalyst::Engine), this is more elegant and would also benefit from being able to have things redacted I guess - as with the current patch, you're going to display the things you're redacting in the logs to the end user... Cheers t0m Tom, Thanks for the feedback. I think you're referring to $c-dump_these() and it's usage in finalize_error(). I'll refactor log_parameters() to call a separate method that will return the params to log, akin to dump_these(). Not sure when I'll have time for it since my current solution is working for me and I have some big deadlines coming up. Hopefully within the next month. Thanks byron ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/ ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/ ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
I've taken a stab at implementing this as I recently was wanting this functionality. See attached for the patch (including docs and unit tests). Feedback welcome. On Fri, Jan 30, 2009 at 5:20 PM, Byron Young byron.yo...@riverbed.comwrote: Tomas Doran wrote on 2009-01-29: On 29 Jan 2009, at 18:53, Byron Young wrote: Hi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff. No, reposting if things get dropped on the floor good :) If you have time, then arriving on #catalyst-dev and making noise also gets stuff done. This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): Having been discussed in #catalyst-dev, we think that the patch could be made both more generic, and more elegant. The key thing is to split the table drawing, and the data filtering into separate methods (maybe filter_debug_data?). This would then allow you to filter per-type, and support things such as redact_parameters (all), redact_body_parameters, redact_query_parameters, and even potentially to add support for filtering things like the URI (I can see use-cases where that'd be significant - e.g. not wanting to log session IDs which are in URIs).. Have a look at the way the debug screen stuff works (in Catalyst::Engine), this is more elegant and would also benefit from being able to have things redacted I guess - as with the current patch, you're going to display the things you're redacting in the logs to the end user... Cheers t0m Tom, Thanks for the feedback. I think you're referring to $c-dump_these() and it's usage in finalize_error(). I'll refactor log_parameters() to call a separate method that will return the params to log, akin to dump_these(). Not sure when I'll have time for it since my current solution is working for me and I have some big deadlines coming up. Hopefully within the next month. Thanks byron ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/ 0001-param-filtering-docs-tests.patch Description: Binary data ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
Sorry for the repost but I realized I hadn't updated to svn HEAD before making the patch file. In case that matters, the attached should apply cleanly against r10759. Thanks! On Wed, Jul 1, 2009 at 11:03 AM, Brian Phillips bpphillips...@gmail.combpphillips%2...@gmail.com wrote: I've taken a stab at implementing this as I recently was wanting this functionality. See attached for the patch (including docs and unit tests). Feedback welcome. On Fri, Jan 30, 2009 at 5:20 PM, Byron Young byron.yo...@riverbed.comwrote: Tomas Doran wrote on 2009-01-29: On 29 Jan 2009, at 18:53, Byron Young wrote: Hi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff. No, reposting if things get dropped on the floor good :) If you have time, then arriving on #catalyst-dev and making noise also gets stuff done. This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): Having been discussed in #catalyst-dev, we think that the patch could be made both more generic, and more elegant. The key thing is to split the table drawing, and the data filtering into separate methods (maybe filter_debug_data?). This would then allow you to filter per-type, and support things such as redact_parameters (all), redact_body_parameters, redact_query_parameters, and even potentially to add support for filtering things like the URI (I can see use-cases where that'd be significant - e.g. not wanting to log session IDs which are in URIs).. Have a look at the way the debug screen stuff works (in Catalyst::Engine), this is more elegant and would also benefit from being able to have things redacted I guess - as with the current patch, you're going to display the things you're redacting in the logs to the end user... Cheers t0m Tom, Thanks for the feedback. I think you're referring to $c-dump_these() and it's usage in finalize_error(). I'll refactor log_parameters() to call a separate method that will return the params to log, akin to dump_these(). Not sure when I'll have time for it since my current solution is working for me and I have some big deadlines coming up. Hopefully within the next month. Thanks byron ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/ 0001-param-filtering-docs-tests.patch Description: Binary data ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
OK, I'm seriously ruining my first attempt at submitting a patch. :-) I changed the capitalization of a config key without updating the unit tests so blush here's another patch file. Apologies for the spam. I think this is the last one I'll need to post on this issue ... hopefully ... :-) On Wed, Jul 1, 2009 at 11:16 AM, Brian Phillips bpphillips...@gmail.combpphillips%2...@gmail.com wrote: Sorry for the repost but I realized I hadn't updated to svn HEAD before making the patch file. In case that matters, the attached should apply cleanly against r10759. Thanks! On Wed, Jul 1, 2009 at 11:03 AM, Brian Phillips bpphillips...@gmail.combpphillips%2...@gmail.com wrote: I've taken a stab at implementing this as I recently was wanting this functionality. See attached for the patch (including docs and unit tests). Feedback welcome. On Fri, Jan 30, 2009 at 5:20 PM, Byron Young byron.yo...@riverbed.comwrote: Tomas Doran wrote on 2009-01-29: On 29 Jan 2009, at 18:53, Byron Young wrote: Hi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff. No, reposting if things get dropped on the floor good :) If you have time, then arriving on #catalyst-dev and making noise also gets stuff done. This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): Having been discussed in #catalyst-dev, we think that the patch could be made both more generic, and more elegant. The key thing is to split the table drawing, and the data filtering into separate methods (maybe filter_debug_data?). This would then allow you to filter per-type, and support things such as redact_parameters (all), redact_body_parameters, redact_query_parameters, and even potentially to add support for filtering things like the URI (I can see use-cases where that'd be significant - e.g. not wanting to log session IDs which are in URIs).. Have a look at the way the debug screen stuff works (in Catalyst::Engine), this is more elegant and would also benefit from being able to have things redacted I guess - as with the current patch, you're going to display the things you're redacting in the logs to the end user... Cheers t0m Tom, Thanks for the feedback. I think you're referring to $c-dump_these() and it's usage in finalize_error(). I'll refactor log_parameters() to call a separate method that will return the params to log, akin to dump_these(). Not sure when I'll have time for it since my current solution is working for me and I have some big deadlines coming up. Hopefully within the next month. Thanks byron ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/ 0001-param-filtering-docs-tests.patch Description: Binary data ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
RE: [Catalyst] Re: Supressing passwords in debug messages
Tomas Doran wrote on 2009-01-29: On 29 Jan 2009, at 18:53, Byron Young wrote: Hi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff. No, reposting if things get dropped on the floor good :) If you have time, then arriving on #catalyst-dev and making noise also gets stuff done. This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): Having been discussed in #catalyst-dev, we think that the patch could be made both more generic, and more elegant. The key thing is to split the table drawing, and the data filtering into separate methods (maybe filter_debug_data?). This would then allow you to filter per-type, and support things such as redact_parameters (all), redact_body_parameters, redact_query_parameters, and even potentially to add support for filtering things like the URI (I can see use-cases where that'd be significant - e.g. not wanting to log session IDs which are in URIs).. Have a look at the way the debug screen stuff works (in Catalyst::Engine), this is more elegant and would also benefit from being able to have things redacted I guess - as with the current patch, you're going to display the things you're redacting in the logs to the end user... Cheers t0m Tom, Thanks for the feedback. I think you're referring to $c-dump_these() and it's usage in finalize_error(). I'll refactor log_parameters() to call a separate method that will return the params to log, akin to dump_these(). Not sure when I'll have time for it since my current solution is working for me and I have some big deadlines coming up. Hopefully within the next month. Thanks byron ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
On Thu, Jan 29, 2009 at 12:30 PM, J. Shirley jshir...@gmail.com wrote:
On Thu, Jan 29, 2009 at 10:53 AM, Byron Young byron.yo...@riverbed.com
wrote:
Hi - I'm not sure what the repost policy on patches, but I have the feeling
this one slipped through the cracks. Let me know if it's generally annoying
to repost stuff.
This is a patch that allows you to suppress printing the value of certain
query or body parameters when running Catalyst in debug mode - For example,
if you want to hide passwords sent from the login page, you can put this in
your app config (yaml):
Debug:
redact_parameters:
- password
and the resulting log will look like:
[debug] Query Parameters are:
.-+--.
| Parameter | Value
|
+-+--+
| password| (redacted by config)
|
| username| some_user
|
'-+--'
There are two patches attached
- redact-patch.diff - contains patch and test
- cookbook-patch.diff - patch for cookbook entry about this
Thanks to J Shirley for help with this.
Thanks
Byron
Byron Young wrote on 2009-01-16:
-Original Message-
From: Byron Young [mailto:byron.yo...@riverbed.com]
Sent: Friday, January 16, 2009 6:39 PM
To: The elegant MVC web framework
Subject: RE: [Catalyst] Re: Supressing passwords in debug messages
Byron Young wrote on 2009-01-12:
J. Shirley wrote on 2009-01-12:
On Mon, Jan 12, 2009 at 2:35 PM, Byron Young
byron.yo...@riverbed.com wrote:
J. Shirley wrote on 2009-01-12:
On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
byron.yo...@riverbed.com wrote:
[snip]
The patch I'm creating needs to be configured in some way, I am
thinking at this point it can be configured as follows:
package MyApp;
__PACKAGE__-config(
'Debug' = {
skip_dump_parameters = 1, # Simply don't render the
parameters incoming, very shotgunny skip_dump_parameters =
[ qw/password/ ], # Show '(redacted
by
config)' as the value of these fields
}
);
I'll need to bake tests for this, which there are currently no tests
for handling the dumping of parameters so it will be a bit more. If
someone wants to help with that, let me know and I can help guide.
-J
I'd be happy to write some unit tests. I haven't worked with
any
of the Catalyst unit tests before so I'm not sure what the process is
like for getting the code, setting up the test environment, making and
submitting changes and unit tests, etc. Is there a doc you can point
me to? I don't see anything in the manual or wiki.
Byron
Mostly it is just checking out the code from svn and starting.
The
patch that I've started is at http://scsys.co.uk:8001/22410 - you can
apply this to a svn checkout of
http://dev.catalystframework.org/repos/Catalyst/Catalyst- Runtime/5.70
It doesn't have the actual testing part, just a stub. I'll be working
on it more over today and tomorrow when I get free moments, but
they're few and far between.
Ditto on the lack of free time. I'll check it out and let you know
what I come up with.
byron
J Shirley - I finally got a chance to look at this today. You did
most of the work for me. I just updated the unit test, changed the
'skip_dump_parameters' parameter to 'redact_parameters', and
expanded the log_parameters() documentation a bit. I also added a
section to the cookbook explaining how to use the parameter.
Attached are two patches:
redact-patch.diff - patch containing the new unit test and changes to
Catalyst.pm. cookbook-patch.diff - patch containing a new cookbook
section on
this feature, for the Catalyst-Manual repository
Anything else I need to do?
Byron
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
Hi Byron,
Just my fault -- been busy and then sick, I'll try to get to it in the
next few days.
-J
Actually, scratch that.
I don't have the tuits or desire to cat herd this out. Someone on the
core team can finish this up with you, I'm out.
-J
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
On 29 Jan 2009, at 18:53, Byron Young wrote: Hi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff. No, reposting if things get dropped on the floor good :) If you have time, then arriving on #catalyst-dev and making noise also gets stuff done. This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): Having been discussed in #catalyst-dev, we think that the patch could be made both more generic, and more elegant. The key thing is to split the table drawing, and the data filtering into separate methods (maybe filter_debug_data?). This would then allow you to filter per-type, and support things such as redact_parameters (all), redact_body_parameters, redact_query_parameters, and even potentially to add support for filtering things like the URI (I can see use-cases where that'd be significant - e.g. not wanting to log session IDs which are in URIs).. Have a look at the way the debug screen stuff works (in Catalyst::Engine), this is more elegant and would also benefit from being able to have things redacted I guess - as with the current patch, you're going to display the things you're redacting in the logs to the end user... Cheers t0m ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
RE: [Catalyst] Re: Supressing passwords in debug messages
Byron Young wrote on 2009-01-12:
J. Shirley wrote on 2009-01-12:
On Mon, Jan 12, 2009 at 2:35 PM, Byron Young
byron.yo...@riverbed.com wrote:
J. Shirley wrote on 2009-01-12:
On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
byron.yo...@riverbed.com wrote:
[snip]
The patch I'm creating needs to be configured in some way, I am
thinking at this point it can be configured as follows:
package MyApp;
__PACKAGE__-config(
'Debug' = {
skip_dump_parameters = 1, # Simply don't render the
parameters incoming, very shotgunny skip_dump_parameters = [
qw/password/ ], # Show '(redacted
by
config)' as the value of these fields
}
);
I'll need to bake tests for this, which there are currently no tests
for handling the dumping of parameters so it will be a bit more. If
someone wants to help with that, let me know and I can help guide.
-J
I'd be happy to write some unit tests. I haven't worked with
any
of the Catalyst unit tests before so I'm not sure what the process is
like for getting the code, setting up the test environment, making and
submitting changes and unit tests, etc. Is there a doc you can point
me to? I don't see anything in the manual or wiki.
Byron
Mostly it is just checking out the code from svn and starting.
The
patch that I've started is at http://scsys.co.uk:8001/22410 - you can
apply this to a svn checkout of
http://dev.catalystframework.org/repos/Catalyst/Catalyst- Runtime/5.70
It doesn't have the actual testing part, just a stub. I'll be working
on it more over today and tomorrow when I get free moments, but they're
few and far between.
Ditto on the lack of free time. I'll check it out and let you know
what I come up with.
byron
J Shirley - I finally got a chance to look at this today. You did most of the
work for me. I just updated the unit test, changed the 'skip_dump_parameters'
parameter to 'redact_parameters', and expanded the log_parameters()
documentation a bit. I also added a section to the cookbook explaining how to
use the parameter.
Attached are two patches:
redact-patch.diff - patch containing the new unit test and changes to
Catalyst.pm.
cookbook-patch.diff - patch containing a new cookbook section on this
feature, for the Catalyst-Manual repository
Anything else I need to do?
Byron
redact-patch.diff
Description: redact-patch.diff
cookbook-patch.diff
Description: cookbook-patch.diff
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
RE: [Catalyst] Re: Supressing passwords in debug messages
Ansgar Burchardt wrote on 2009-01-11:
Hi,
J. Shirley jshir...@gmail.com writes:
=== lib/Catalyst.pm
== ---
lib/Catalyst.pm (revision 18145) +++ lib/Catalyst.pm (local) @@
-1830,7 +1830,11 @@
if ( $c-debug keys %{ $c-request-query_parameters } )
{
my $t = Text::SimpleTable-new( [ 35, 'Parameter' ], [
36, 'Value' ] );
+my %skip = map { $_ = $_ } @{
+$c-config-{'Plugin::Debug'}-
{'skip_dump_parameters'} || []
+};
for my $key ( sort keys %{ $c-req-query_parameters } )
{
+next if $skip{$key};
my $param = $c-req-query_parameters-{$key};
my $value = defined($param) ? $param : '';
$t-row( $key,
I think it would be better to show that the parameter was sent, but
Catalyst configured to not display its value. This can be done for
example by displaying a value of `(hidden)'.
If the parameter is simply skipped, it might be confusing if you forget
that you configured Catalyst to not display it.
Regards,
Ansgar
Yeah, I agree that the parameter should be shown as sent, but just not show the
value.
J Shirley - Thanks for looking into it. Let me know if there's anything I can
do to help.
Thanks,
Byron
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
On Mon, Jan 12, 2009 at 10:45 AM, Byron Young byron.yo...@riverbed.com wrote:
Ansgar Burchardt wrote on 2009-01-11:
Hi,
J. Shirley jshir...@gmail.com writes:
=== lib/Catalyst.pm
== ---
lib/Catalyst.pm (revision 18145) +++ lib/Catalyst.pm (local) @@
-1830,7 +1830,11 @@
if ( $c-debug keys %{ $c-request-query_parameters } )
{
my $t = Text::SimpleTable-new( [ 35, 'Parameter' ], [
36, 'Value' ] );
+my %skip = map { $_ = $_ } @{
+$c-config-{'Plugin::Debug'}-
{'skip_dump_parameters'} || []
+};
for my $key ( sort keys %{ $c-req-query_parameters } )
{
+next if $skip{$key};
my $param = $c-req-query_parameters-{$key};
my $value = defined($param) ? $param : '';
$t-row( $key,
I think it would be better to show that the parameter was sent, but
Catalyst configured to not display its value. This can be done for
example by displaying a value of `(hidden)'.
If the parameter is simply skipped, it might be confusing if you forget
that you configured Catalyst to not display it.
Regards,
Ansgar
Yeah, I agree that the parameter should be shown as sent, but just not show
the value.
J Shirley - Thanks for looking into it. Let me know if there's anything I
can do to help.
Thanks,
Byron
The patch I'm creating needs to be configured in some way, I am
thinking at this point it can be configured as follows:
package MyApp;
__PACKAGE__-config(
'Debug' = {
skip_dump_parameters = 1, # Simply don't render the
parameters incoming, very shotgunny
skip_dump_parameters = [ qw/password/ ], # Show '(redacted by
config)' as the value of these fields
}
);
I'll need to bake tests for this, which there are currently no tests
for handling the dumping of parameters so it will be a bit more. If
someone wants to help with that, let me know and I can help guide.
-J
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
RE: [Catalyst] Re: Supressing passwords in debug messages
J. Shirley wrote on 2009-01-12:
On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
byron.yo...@riverbed.com wrote:
Ansgar Burchardt wrote on 2009-01-11:
Hi,
J. Shirley jshir...@gmail.com writes:
=== lib/Catalyst.pm
==
---
lib/Catalyst.pm (revision 18145) +++ lib/Catalyst.pm (local) @@
-1830,7 +1830,11 @@
if ( $c-debug keys %{ $c-request-query_parameters }
)
{
my $t = Text::SimpleTable-new( [ 35, 'Parameter' ], [
36, 'Value' ] );
+my %skip = map { $_ = $_ } @{
+$c-config-{'Plugin::Debug'}-
{'skip_dump_parameters'} || []
+};
for my $key ( sort keys %{ $c-req-query_parameters }
)
{
+next if $skip{$key};
my $param = $c-req-query_parameters-{$key};
my $value = defined($param) ? $param : '';
$t-row( $key,
I think it would be better to show that the parameter was sent, but
Catalyst configured to not display its value. This can be done for
example by displaying a value of `(hidden)'.
If the parameter is simply skipped, it might be confusing if you
forget that you configured Catalyst to not display it.
Regards,
Ansgar
Yeah, I agree that the parameter should be shown as sent, but just not
show the value.
J Shirley - Thanks for looking into it. Let me know if there's
anything I can do to help.
Thanks,
Byron
The patch I'm creating needs to be configured in some way, I am
thinking at this point it can be configured as follows:
package MyApp;
__PACKAGE__-config(
'Debug' = {
skip_dump_parameters = 1, # Simply don't render the parameters
incoming, very shotgunny skip_dump_parameters = [ qw/password/
], # Show '(redacted
by
config)' as the value of these fields
}
);
I'll need to bake tests for this, which there are currently no tests for
handling the dumping of parameters so it will be a bit more. If someone
wants to help with that, let me know and I can help guide.
-J
I'd be happy to write some unit tests. I haven't worked with any of the
Catalyst unit tests before so I'm not sure what the process is like for getting
the code, setting up the test environment, making and submitting changes and
unit tests, etc. Is there a doc you can point me to? I don't see anything in
the manual or wiki.
Byron
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Re: Supressing passwords in debug messages
On Mon, Jan 12, 2009 at 2:35 PM, Byron Young byron.yo...@riverbed.com wrote:
J. Shirley wrote on 2009-01-12:
On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
byron.yo...@riverbed.com wrote:
Ansgar Burchardt wrote on 2009-01-11:
Hi,
J. Shirley jshir...@gmail.com writes:
=== lib/Catalyst.pm
==
---
lib/Catalyst.pm (revision 18145) +++ lib/Catalyst.pm (local) @@
-1830,7 +1830,11 @@
if ( $c-debug keys %{ $c-request-query_parameters }
)
{
my $t = Text::SimpleTable-new( [ 35, 'Parameter' ], [
36, 'Value' ] );
+my %skip = map { $_ = $_ } @{
+$c-config-{'Plugin::Debug'}-
{'skip_dump_parameters'} || []
+};
for my $key ( sort keys %{ $c-req-query_parameters }
)
{
+next if $skip{$key};
my $param = $c-req-query_parameters-{$key};
my $value = defined($param) ? $param : '';
$t-row( $key,
I think it would be better to show that the parameter was sent, but
Catalyst configured to not display its value. This can be done for
example by displaying a value of `(hidden)'.
If the parameter is simply skipped, it might be confusing if you
forget that you configured Catalyst to not display it.
Regards,
Ansgar
Yeah, I agree that the parameter should be shown as sent, but just not
show the value.
J Shirley - Thanks for looking into it. Let me know if there's
anything I can do to help.
Thanks,
Byron
The patch I'm creating needs to be configured in some way, I am
thinking at this point it can be configured as follows:
package MyApp;
__PACKAGE__-config(
'Debug' = {
skip_dump_parameters = 1, # Simply don't render the parameters
incoming, very shotgunny skip_dump_parameters = [ qw/password/
], # Show '(redacted
by
config)' as the value of these fields
}
);
I'll need to bake tests for this, which there are currently no tests for
handling the dumping of parameters so it will be a bit more. If someone
wants to help with that, let me know and I can help guide.
-J
I'd be happy to write some unit tests. I haven't worked with any of the
Catalyst unit tests before so I'm not sure what the process is like for
getting the code, setting up the test environment, making and submitting
changes and unit tests, etc. Is there a doc you can point me to? I don't
see anything in the manual or wiki.
Byron
Mostly it is just checking out the code from svn and starting. The
patch that I've started is at http://scsys.co.uk:8001/22410 - you can
apply this to a svn checkout of
http://dev.catalystframework.org/repos/Catalyst/Catalyst-Runtime/5.70
It doesn't have the actual testing part, just a stub. I'll be working
on it more over today and tomorrow when I get free moments, but
they're few and far between.
-J
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
RE: [Catalyst] Re: Supressing passwords in debug messages
J. Shirley wrote on 2009-01-12:
On Mon, Jan 12, 2009 at 2:35 PM, Byron Young
byron.yo...@riverbed.com wrote:
J. Shirley wrote on 2009-01-12:
On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
byron.yo...@riverbed.com wrote:
[snip]
The patch I'm creating needs to be configured in some way, I am
thinking at this point it can be configured as follows:
package MyApp;
__PACKAGE__-config(
'Debug' = {
skip_dump_parameters = 1, # Simply don't render the
parameters incoming, very shotgunny skip_dump_parameters = [
qw/password/ ], # Show '(redacted
by
config)' as the value of these fields
}
);
I'll need to bake tests for this, which there are currently no tests
for handling the dumping of parameters so it will be a bit more. If
someone wants to help with that, let me know and I can help guide.
-J
I'd be happy to write some unit tests. I haven't worked with any
of the Catalyst unit tests before so I'm not sure what the process
is like for getting the code, setting up the test environment,
making and submitting changes and unit tests, etc. Is there a doc
you can point me to? I don't see anything in the manual or wiki.
Byron
Mostly it is just checking out the code from svn and starting. The
patch that I've started is at http://scsys.co.uk:8001/22410 - you can
apply this to a svn checkout of
http://dev.catalystframework.org/repos/Catalyst/Catalyst- Runtime/5.70
It doesn't have the actual testing part, just a stub. I'll be working
on it more over today and tomorrow when I get free moments, but they're
few and far between.
Ditto on the lack of free time. I'll check it out and let you know what I come
up with.
byron
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
