From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Wed, 27 Feb 2013 14:18:46 GMT
Subject: Re: [PATCHv4 5/6] Git.pm: add interface for git credential command
Message-ID: <vpqppzl4z7d.fsf@grenoble-inp.fr>
In-Reply-To: <e5834c9b5ccb66a88b64b3d07982ad41205fb97e.1360677646.git.mina86@mina86.com>

Michal Nazarewicz <mpn@google.com> writes:

> +=item credential_read( FILEHANDLE )
> +
> +Reads credential key-value pairs from C<FILEHANDLE>.  Reading stops at EOF or
> +when an empty line is encountered.  Each line must be of the form C<key=value>
> +with a non-empty key.  Function returns hash with all read values.  Any white
> +space (other then new-line character) is preserved.

Typo: other then -> than.

> +sub credential_read {
> +	my ($self, $reader) = _maybe_self(@_);
> +	my %credential;
> +	while (<$reader>) {
> +		chomp;
> +		if ($_ eq '') {
> +			last;
> +		} elsif (!/^([^=]+)=(.*)$/) {

Good.

> +	# Check if $credential is valid prior to writing anything
> +	while (($key, $value) = each %$credential) {
> +		if (!defined $key || !length $key) {
> +			throw Error::Simple("credential key empty or undefined");
> +		} elsif ($key =~ /[=\n\0]/) {
> +			throw Error::Simple("credential key contains invalid characters: $key");
> +		} elsif (defined $value && $value =~ /[\n\0]/) {
> +			throw Error::Simple("credential value for key=$key contains invalid characters: $value");
> +		}
> +	}

Good.

These checks seem to address all the points raised during discussion
about when the API should reject values.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/