launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'lib/lp/snappy/model/snapstoreclient.py'
> --- lib/lp/snappy/model/snapstoreclient.py	2016-05-31 10:54:32 +0000
> +++ lib/lp/snappy/model/snapstoreclient.py	2016-06-02 11:23:30 +0000
> @@ -65,8 +69,12 @@
>      @classmethod
>      def _makeAuthParam(cls, key, value):
>          # Check framing.
> -        assert set(key).issubset(cls.allowed_chars)
> -        assert set(value).issubset(cls.allowed_chars)
> +        if not set(key).issubset(cls.allowed_chars):
> +            raise InvalidStoreSecretsError(
> +                "Key contains unsafe characters: %r" % key)
> +        if not set(value).issubset(cls.allowed_chars):
> +            raise InvalidStoreSecretsError(
> +                "Value contains unsafe characters: %r" % value)

Is it wise to include secrets in exceptions?

>          return '%s="%s"' % (key, value)
>  
>      @property


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-improve-framing-checks/+merge/296312
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.