On 9/9/14 11:01 AM, I wrote:
On 9/9/14 10:54 AM, Heikki Linnakangas wrote:
So I think this (and the corresponding dearmor code too) should be
refactored to use a StringInfo that gets enlarged as needed, instead of
hoping to guess the size correctly beforehand. To ease review, might
make sense to do that as a separate patch over current sources, and the
main patch on top of that.
Yeah, I thought the same thing while writing that code. Thanks, I'll do
it that way.
Attached is what I have right now. I started working on the decoding
part, but it has this piece of code:
/* decode crc */
if (b64_decode(p + 1, 4, buf) != 3)
goto out;
which makes the approach a bit uglier. If I did this the same way, I
would have to create and destroy a StringInfo just for this operation,
which seems ugly. So I wonder if I shouldn't try and instead keep the
code closer to what it is in HEAD right now; I could call
enlargeStringInfo() first, then hand out a pointer to b64_encode (or
b64_decode()) and finally increment StringInfoData.len by how much was
actually written. That would keep the code changes a lot smaller, too.
Is either of these approaches anywhere near what you had in mind?
I'm also not sure why we need to keep a copy of the base64
encoding/decoding logic instead of exporting it in utils/adt/encode.c.
.marko
*** a/contrib/pgcrypto/pgp-armor.c
--- b/contrib/pgcrypto/pgp-armor.c
***************
*** 31,36 ****
--- 31,38 ----
#include "postgres.h"
+ #include "lib/stringinfo.h"
+
#include "px.h"
#include "pgp.h"
***************
*** 38,58 ****
* BASE64 - duplicated :(
*/
! static const unsigned char _base64[] =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
! static int
! b64_encode(const uint8 *src, unsigned len, uint8 *dst)
{
- uint8 *p,
- *lend = dst + 76;
const uint8 *s,
*end = src + len;
int pos = 2;
unsigned long buf = 0;
s = src;
- p = dst;
while (s < end)
{
--- 40,58 ----
* BASE64 - duplicated :(
*/
! static const char _base64[] =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
! static void
! b64_encode(const uint8 *src, unsigned len, StringInfo dst)
{
const uint8 *s,
*end = src + len;
int pos = 2;
unsigned long buf = 0;
+ int linepos = 0;
s = src;
while (s < end)
{
***************
*** 65,93 **** b64_encode(const uint8 *src, unsigned len, uint8 *dst)
*/
if (pos < 0)
{
! *p++ = _base64[(buf >> 18) & 0x3f];
! *p++ = _base64[(buf >> 12) & 0x3f];
! *p++ = _base64[(buf >> 6) & 0x3f];
! *p++ = _base64[buf & 0x3f];
pos = 2;
buf = 0;
}
! if (p >= lend)
{
! *p++ = '\n';
! lend = p + 76;
}
}
if (pos != 2)
{
! *p++ = _base64[(buf >> 18) & 0x3f];
! *p++ = _base64[(buf >> 12) & 0x3f];
! *p++ = (pos == 0) ? _base64[(buf >> 6) & 0x3f] : '=';
! *p++ = '=';
}
-
- return p - dst;
}
/* probably should use lookup table */
--- 65,92 ----
*/
if (pos < 0)
{
! appendStringInfoCharMacro(dst, _base64[(buf >> 18) &
0x3f]);
! appendStringInfoCharMacro(dst, _base64[(buf >> 12) &
0x3f]);
! appendStringInfoCharMacro(dst, _base64[(buf >> 6) &
0x3f]);
! appendStringInfoCharMacro(dst, _base64[buf & 0x3f]);
+ linepos += 4;
pos = 2;
buf = 0;
}
! if (linepos >= 76)
{
! appendStringInfoCharMacro(dst, '\n');
! linepos = 0;
}
}
if (pos != 2)
{
! appendStringInfoCharMacro(dst, _base64[(buf >> 18) & 0x3f]);
! appendStringInfoCharMacro(dst, _base64[(buf >> 12) & 0x3f]);
! appendStringInfoCharMacro(dst, (pos == 0) ? _base64[(buf >> 6)
& 0x3f] : '=');
! appendStringInfoCharMacro(dst, '=');
}
}
/* probably should use lookup table */
***************
*** 160,174 **** b64_decode(const uint8 *src, unsigned len, uint8 *dst)
}
static unsigned
- b64_enc_len(unsigned srclen)
- {
- /*
- * 3 bytes will be converted to 4, linefeed after 76 chars
- */
- return (srclen + 2) * 4 / 3 + srclen / (76 * 3 / 4);
- }
-
- static unsigned
b64_dec_len(unsigned srclen)
{
return (srclen * 3) >> 2;
--- 159,164 ----
***************
*** 203,240 **** crc24(const uint8 *data, unsigned len)
return crc & 0xffffffL;
}
! int
! pgp_armor_encode(const uint8 *src, unsigned len, uint8 *dst)
{
- int n;
- uint8 *pos = dst;
unsigned crc = crc24(src, len);
! n = strlen(armor_header);
! memcpy(pos, armor_header, n);
! pos += n;
! n = b64_encode(src, len, pos);
! pos += n;
! if (*(pos - 1) != '\n')
! *pos++ = '\n';
! *pos++ = '=';
! pos[3] = _base64[crc & 0x3f];
! crc >>= 6;
! pos[2] = _base64[crc & 0x3f];
! crc >>= 6;
! pos[1] = _base64[crc & 0x3f];
! crc >>= 6;
! pos[0] = _base64[crc & 0x3f];
! pos += 4;
! n = strlen(armor_footer);
! memcpy(pos, armor_footer, n);
! pos += n;
!
! return pos - dst;
}
static const uint8 *
--- 193,217 ----
return crc & 0xffffffL;
}
! void
! pgp_armor_encode(const uint8 *src, unsigned len, StringInfo dst)
{
unsigned crc = crc24(src, len);
! appendStringInfoString(dst, armor_header);
! b64_encode(src, len, dst);
! if (*(dst->data + dst->len - 1) != '\n')
! appendStringInfoChar(dst, '\n');
! appendStringInfoChar(dst, '=');
! appendStringInfoChar(dst, _base64[(crc >> 18) & 0x3f]);
! appendStringInfoChar(dst, _base64[(crc >> 12) & 0x3f]);
! appendStringInfoChar(dst, _base64[(crc >> 6) & 0x3f]);
! appendStringInfoChar(dst, _base64[crc & 0x3f]);
! appendStringInfoString(dst, armor_footer);
}
static const uint8 *
***************
*** 370,381 **** out:
}
unsigned
- pgp_armor_enc_len(unsigned len)
- {
- return b64_enc_len(len) + strlen(armor_header) + strlen(armor_footer) +
16;
- }
-
- unsigned
pgp_armor_dec_len(unsigned len)
{
return b64_dec_len(len);
--- 347,352 ----
*** a/contrib/pgcrypto/pgp-pgsql.c
--- b/contrib/pgcrypto/pgp-pgsql.c
***************
*** 820,842 **** pg_armor(PG_FUNCTION_ARGS)
{
bytea *data;
text *res;
! int data_len,
! res_len,
! guess_len;
data = PG_GETARG_BYTEA_P(0);
data_len = VARSIZE(data) - VARHDRSZ;
! guess_len = pgp_armor_enc_len(data_len);
! res = palloc(VARHDRSZ + guess_len);
! res_len = pgp_armor_encode((uint8 *) VARDATA(data), data_len,
! (uint8 *)
VARDATA(res));
! if (res_len > guess_len)
! ereport(ERROR,
!
(errcode(ERRCODE_EXTERNAL_ROUTINE_INVOCATION_EXCEPTION),
! errmsg("Overflow - encode estimate too
small")));
! SET_VARSIZE(res, VARHDRSZ + res_len);
PG_FREE_IF_COPY(data, 0);
PG_RETURN_TEXT_P(res);
--- 820,839 ----
{
bytea *data;
text *res;
! int data_len;
! StringInfoData buf;
data = PG_GETARG_BYTEA_P(0);
data_len = VARSIZE(data) - VARHDRSZ;
! initStringInfo(&buf);
! pgp_armor_encode((uint8 *) VARDATA(data), data_len, &buf);
!
! res = palloc(VARHDRSZ + buf.len);
! SET_VARSIZE(res, VARHDRSZ + buf.len);
! memcpy(VARDATA(res), buf.data, buf.len);
! pfree(buf.data);
PG_FREE_IF_COPY(data, 0);
PG_RETURN_TEXT_P(res);
*** a/contrib/pgcrypto/pgp.h
--- b/contrib/pgcrypto/pgp.h
***************
*** 29,34 ****
--- 29,35 ----
* contrib/pgcrypto/pgp.h
*/
+ #include "lib/stringinfo.h"
#include "mbuf.h"
#include "px.h"
***************
*** 274,280 **** void pgp_cfb_free(PGP_CFB *ctx);
int pgp_cfb_encrypt(PGP_CFB *ctx, const uint8 *data, int
len, uint8 *dst);
int pgp_cfb_decrypt(PGP_CFB *ctx, const uint8 *data, int
len, uint8 *dst);
! int pgp_armor_encode(const uint8 *src, unsigned len, uint8
*dst);
int pgp_armor_decode(const uint8 *src, unsigned len, uint8
*dst);
unsigned pgp_armor_enc_len(unsigned len);
unsigned pgp_armor_dec_len(unsigned len);
--- 275,281 ----
int pgp_cfb_encrypt(PGP_CFB *ctx, const uint8 *data, int
len, uint8 *dst);
int pgp_cfb_decrypt(PGP_CFB *ctx, const uint8 *data, int
len, uint8 *dst);
! void pgp_armor_encode(const uint8 *src, unsigned len, StringInfo
dst);
int pgp_armor_decode(const uint8 *src, unsigned len, uint8
*dst);
unsigned pgp_armor_enc_len(unsigned len);
unsigned pgp_armor_dec_len(unsigned len);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
