[Catalyst] Re: Supressing passwords in debug messages

[J. Shirley]

On Mon, Jan 12, 2009 at 2:35 PM, Byron Young <Byron.Young at riverbed.com> wrote:
> J. Shirley wrote on 2009-01-12:
>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
>> <Byron.Young at riverbed.com> wrote:
>>> Ansgar Burchardt wrote on 2009-01-11:
>>>> Hi,
>>>>
>>>> "J. Shirley" <jshirley at 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