On 10/1/14 1:01 PM, Heikki Linnakangas wrote:
On 10/01/2014 11:58 AM, Marko Tiikkaja wrote:
On 10/1/14, 9:11 AM, Heikki Linnakangas wrote:
We have two options:

1. Throw an error if there are any non-ASCII characters in the key/value
arrays.
2. Don't convert them to UTF-8, but use the current database encoding.

Both seem sane to me. If we use the current database encoding, then we
have to also decide what to do with the input, in pgp_armor_headers().
If armor() uses the database encoding, but pgp_armor_headers() treats
the input as UTF-8, then a round-trip with pgp_armor_headers(armor(?))
won't work.

Yeah.  Both options seem fine to me.  Throwing an error perhaps slightly
more so.

I went with 1, throw an error. I also added checks that the key or value
doesn't contain any embedded newlines, and that the key doesn't contain
an embedded ": ". Those would cause the armor to be invalid.

Great.

I think this is now ready for commit, but since I've changed it quite
significantly from what you originally submitted, please take a moment
to review this.

  1) I see this compiler warning:

    pgp-pgsql.c: In function ‘pg_armor’:
pgp-pgsql.c:960:18: warning: ‘values’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    pgp_armor_encode((uint8 *) VARDATA(data), data_len, &buf,

     It's bogus, but worth silencing anyway.

2) There's what looks like an extra whitespace change in pgp_armor_encode(), but maybe that's intentional?

  3) Also, I think the attached two corner cases deserve their own tests.

Other than the above, the patch looks good to me. Huge thanks for your work on this one!


.marko
*** a/contrib/pgcrypto/expected/pgp-armor.out
--- b/contrib/pgcrypto/expected/pgp-armor.out
***************
*** 112,117 **** em9va2E=
--- 112,137 ----
  -----END PGP MESSAGE-----
  ');
  ERROR:  Corrupt ascii-armor
+ -- corrupt (no empty line)
+ select * from pgp_armor_headers('
+ -----BEGIN PGP MESSAGE-----
+ em9va2E=
+ =ZZZZ
+ -----END PGP MESSAGE-----
+ ');
+ ERROR:  Corrupt ascii-armor
+ -- no headers
+ select * from pgp_armor_headers('
+ -----BEGIN PGP MESSAGE-----
+ 
+ em9va2E=
+ =ZZZZ
+ -----END PGP MESSAGE-----
+ ');
+  key | value 
+ -----+-------
+ (0 rows)
+ 
  -- header with empty value
  select * from pgp_armor_headers('
  -----BEGIN PGP MESSAGE-----
*** a/contrib/pgcrypto/sql/pgp-armor.sql
--- b/contrib/pgcrypto/sql/pgp-armor.sql
***************
*** 67,72 **** em9va2E=
--- 67,89 ----
  -----END PGP MESSAGE-----
  ');
  
+ -- corrupt (no empty line)
+ select * from pgp_armor_headers('
+ -----BEGIN PGP MESSAGE-----
+ em9va2E=
+ =ZZZZ
+ -----END PGP MESSAGE-----
+ ');
+ 
+ -- no headers
+ select * from pgp_armor_headers('
+ -----BEGIN PGP MESSAGE-----
+ 
+ em9va2E=
+ =ZZZZ
+ -----END PGP MESSAGE-----
+ ');
+ 
  -- header with empty value
  select * from pgp_armor_headers('
  -----BEGIN PGP MESSAGE-----
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to