Postgresql corrupt data in input function of custom base data type

Question

I've created a custom type, gp to model the DND 5e currency system. I have defined custom input and output functions in gp.c:

#include "postgres.h"
#include <string.h>
#include "fmgr.h"

#include <stdio.h>

#ifdef PG_MODULE_MAGIC
PG_MODULE_MAGIC;
#endif

static const char* inputFormat = " %i %s2 ";
static const char* invalidFormat = "invalid input syntax for gp: \"%s\"";

PG_FUNCTION_INFO_V1(gp_input);

Datum gp_input(PG_FUNCTION_ARGS) {
    char* raw = PG_GETARG_CSTRING(0);
    int32 amt;
    char unit[3];

    if (sscanf(raw, inputFormat, &amt, &unit[0]) != 2) {
        ereport(ERROR, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg(invalidFormat, raw)));
    }

    switch(unit[1]) {
        case 'p':
            break;
        default:
            ereport(ERROR, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg(invalidFormat, raw)));
    }
    switch(unit[0]) {
        case 'c':
            break;
        case 's':
            amt *= 10;
            break;
        case 'e':
            amt *= 50;
            break;
        case 'g':
            amt *= 100;
            break;
        case 'p':
            amt *= 1000;
            break;
        default:
            ereport(ERROR, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg(invalidFormat, raw)));
    }

    int32* result = (int32*)palloc(sizeof(int32));
    *result = amt;

    PG_RETURN_POINTER(result);
}

PG_FUNCTION_INFO_V1(gp_output);

Datum gp_output(PG_FUNCTION_ARGS) {
    int32* raw = (int32*)PG_GETARG_POINTER(0);
    int32 val = *raw;
    unsigned int bufsz = sizeof(unsigned char)*9 + 2;// allow up to 999999999[pgsc]p
    char* buf = (char*) palloc(bufsz+1); // +1 b/c '\0'

    if (val >= 10 && val % 10 == 0) {
        val /= 10;

        if (val >= 10 && val % 10 == 0) {
            val /= 10;

            if (val >= 10 && val % 10 == 0) {
                val /= 10;

                if (sprintf(buf, "%dpp", val) <= 0) {
                    ereport(ERROR, (errcode(ERRCODE_UNTRANSLATABLE_CHARACTER), errmsg("Bad value for gp")));
                }
            }
            else {
                if (sprintf(buf, "%dgp", val) <= 0) {
                    ereport(ERROR, (errcode(ERRCODE_UNTRANSLATABLE_CHARACTER), errmsg("Bad value for gp")));
                }
            }
        }
        else {
            if (sprintf(buf, "%dsp", val) <= 0) {
                ereport(ERROR, (errcode(ERRCODE_UNTRANSLATABLE_CHARACTER), errmsg("Bad value for gp")));
            }
        }
    }
    else {
        if (sprintf(buf, "%dcp", val) <= 0) {
            ereport(ERROR, (errcode(ERRCODE_UNTRANSLATABLE_CHARACTER), errmsg("Bad value for gp")));
        }
    }
    PG_RETURN_CSTRING(buf);
}

I know I'm not checking that the number is out-of-bounds or that the stored value fits in the buffer, but I'm not hitting that issue yet. My problem is that postgres seems to be editing, and in some cases corrupting, the values that I'm storing. I have this test SQL file:

DROP TYPE IF EXISTS gp CASCADE;
DROP TABLE IF EXISTS test;

CREATE TYPE gp;

CREATE FUNCTION gp_input(cstring) RETURNS gp AS '$libdir/gp.so' LANGUAGE C IMMUTABLE STRICT;
CREATE FUNCTION gp_output(gp) RETURNS cstring AS '$libdir/gp.so' LANGUAGE C IMMUTABLE STRICT;

CREATE TYPE gp (input=gp_input, output=gp_output);

CREATE TABLE test (val gp);

INSERT INTO test VALUES ('12sp'), ('100gp'), ('1000cp'), ('101cp');
SELECT * FROM test;
INSERT INTO test VALUES ('101sp');

The output of that SELECT is:

  val  
-------
 12sp
 10pp
 1pp
 212cp
(4 rows)

So we can see that all values were properly stored and represented except for the last one: 101cp gets stored as a pointer to the int32 value 212. Using ereport warnings, I was able to determine that right before the return in the input function, result points to the correct value: 101. However, the pointer passed as an argument to my output function points to a value I didn't store: 212. Somewhere between the end of my input code and the beginning of my output code, postgres corrupted that value. This always happens with the input string 101cp, independent of the table's state or any other values being inserted at the same time.

But now the really weird part; that last INSERT crashes the client. Upon parsing that gp value, it prints the error:

psql:./gptest.sql:15: ERROR:  compressed data is corrupted
LINE 1: INSERT INTO test VALUES ('101sp');
                                 ^

This always happens with the value 101sp, regardless of table state or any other values being inserted alongside it. Using ereport warnings, I was able to see that right before the return statement, result points to the correct value: 1010. That also means that the crash is happening in that return macro expansion or in some under-the-hood code.

So I truly have no idea what's going on. I'm doing palloc so overwriting the memory shouldn't be allowed, and I can't think of any reason for values containing 101 to always have problems - and different problems depending on the units. An int32 should be capable of storing the small values I'm testing, so it's not that. Idk if this is how it's supposed to be implemented, but I have checked and the pointer being passed to the output is NOT the same as the address of the result pointer for any of these values, so I assume it's doing some kind of memcpy incorrectly under the hood, but then idk how anyone can be expected to define a custom base data type.

Nick Barnes · Accepted Answer · 2019-02-14 15:04:50Z

1

CREATE TYPE takes a ton of optional parameters, some of them relating to the physical layout of your data, and these need to agree with the structures your I/O functions are expecting/returning.

The docs don't seem to mention the defaults for these parameters, but the error mentioning "compressed data" suggests that your values are being TOASTed, i.e. that INTERNALLENGTH is defaulting to VARIABLE. Such types are expected to start with a varlena header describing the total length of the value, which is certainly not what you're returning (though Postgres would still interpret it as such, leading to all kinds of odd behaviour, not to mention saving huge slabs of random bytes to your table, and probably segfaulting sooner or later...).

If your aim is to create a type which is internally represented as a simple integer (fixed-length, pass-by-value, etc.), you can just copy the characteristics of the built-in type:

CREATE TYPE gp (input=gp_input, output=gp_output, like=integer);

You should then be able to do away with the palloc() and the pointers, fetch your argument with PG_GETARG_INT32(0), and just return PG_RETURN_INT32(amt).

If you want all of the behaviour of a built-in type, but with a custom display format, it's a lot easier than you might expect.

The internal C routines for something like numeric are identical to the ones you'd write to implement such a type on your own. As as result, you can create your own version of such a built-in type simply by copy-pasting its SQL-level definitions, and leaving the functions pointing to the existing C handlers to do all of the actual work:

CREATE TYPE gp;
CREATE FUNCTION gp_in(cstring,oid,integer) RETURNS gp LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'numeric_in';
CREATE FUNCTION gp_out(gp) RETURNS cstring LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'numeric_out';
CREATE FUNCTION gp_send(gp) RETURNS bytea LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'numeric_send';
CREATE FUNCTION gp_recv(internal,oid,integer) RETURNS gp LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'numeric_recv';
CREATE FUNCTION gptypmodin(cstring[]) RETURNS integer LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'numerictypmodin';
CREATE FUNCTION gptypmodout(integer) RETURNS cstring LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'numerictypmodout';
CREATE TYPE gp (
  INPUT = gp_in,
  OUTPUT = gp_out,
  RECEIVE = gp_recv,
  SEND = gp_send,
  TYPMOD_IN = gptypmodin,
  TYPMOD_OUT = gptypmodout,
  LIKE = numeric
);

CREATE TABLE t (x gp(10,2), y gp);
INSERT INTO t VALUES ('123.45', '2387456987623498765324.2837654987364987269837456981');
SELECT * FROM t;

   x    |                          y
--------+-----------------------------------------------------
 123.45 | 2387456987623498765324.2837654987364987269837456981

From there, you can replace the input/output handlers with your own C functions, copy-pasting the code from the internal functions as a starting point. The simplest approach in your case is probably to transform your DnD currency string into a simple decimal string at the start of the function, and let the rest of the code worry about the messy details of converting it to a Numeric.

If you want arithmetic/comparison operators, index opclasses, min/max aggregates, typecasts, etc., you can copy-paste those definitions from the original type as well, as long as you don't mess with the internal binary format.

edited Feb 14, 2019 at 15:04

answered Feb 13, 2019 at 17:47

Nick Barnes

21.8k5 gold badges63 silver badges70 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

ocket8888 Over a year ago

Thanks. Ideally, to mimic money, this would be a variable-sized numeric, but accomplishing that seems very complicated and not well-documented. So I guess for the moment, at least, I'll need to settle for fixed-size integral values. It'd also be neat if I had any idea why these specific values had these problems.

Nick Barnes Over a year ago

Too much to fit in a comment; see the updated answer. money is fixed-length (8 bytes), so if you want variable-length values, you might want a numeric (I'm sure that someone in the history of D&D has managed to get hold of more than 2^64cp...).

Nick Barnes Over a year ago

You're right, there's almost nothing in the HTML docs about these low-level implementation details. But the source code is thoroughly commented, and since they build so much of the core functionality on top of their own APIs, there is usually no better explanation than the code itself. For pretty much anything you might want to do, you can find an example to imitate.

Nick Barnes Over a year ago

As for why specific values triggered this odd behaviour, I'm guessing it's because the TOAST mechanism hijacks a couple of varlena header bits indicate the storage mode, and within certain size ranges, it'll flip them fairly predictably. You might see them flip-flop around the boundaries, depending on the compressibility of the subsequent chunk of memory. But this is all guesswork based on your error message; I've never really had to dig in to the implementation.

ocket8888 Over a year ago

Thanks, that's a great suggestion. I suppose it should be variable length, because you're right, somebody probably has gotten hold of more than that amount of money - or at least more than that in total including gear value. I would like to avoid making it a decimal, though that may not be realistic because I just found out last night that the price per unit for Sling Bullets is 0.2cp. I have no idea how you'd actually pay someone in-game that amount, since 1cp is the smallest denomination, but...

|

Collectives™ on Stack Overflow

Postgresql corrupt data in input function of custom base data type

1 Answer 1

7 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

7 Comments

Your Answer

Sign up or log in

Post as a guest

Related