Обсуждение: It's seems that the function "do_text_output_multiline" does not suit for format "line1\nline2\n...lineN".
Hi all,
Today, I am do some works on adding some customized featues to PostgreSQL 9.6 beta1. But, when i do some output to psql using the fuction "do_text_output_multiline" with the string just like mentioned in mail tilte, such as "this is a\ntest for\nnew blank.". the PostgreSQL may lead to corruption in this function, and i debugged it that found this function can not dealt with the boundaries properly. The original function code as :do_text_output_multiline(TupOutputState *tstate, char *text)
{
Datum values[1];
bool isnull[1] = {false};
while (*text)
{
char *eol;
int len;
eol = strchr(text, '\n');
if (eol)
{
len = eol - text;
eol++;
}
else
{
len = strlen(text);
eol += len;
}
values[0] = PointerGetDatum(cstring_to_text_with_len(text, len));
do_tup_output(tstate, values, isnull);
pfree(DatumGetPointer(values[0]));
text = eol;
}
}
using the string "this is a\ntest for\nnew blank." as the second input parameter, when dealing with the the last part of stirng, "new blank.", the code "eol = strchr(text, '\n');" will return NULL and will go into the "else" part, and the varialbe "eol" will added an integer, which is the length of "new blank.", eol will be 0xa, text is set to 0xa too. this address will is a system used address, then corruption. I think some boundary check will be added as following.
do_text_output_multiline(TupOutputState *tstate, char *text)
{
Datum values[1];
bool isnull[1] = {false};
while (text && *text) //if the text is NULL the return, do nothing.
{
char *eol;
int len;
eol = strchr(text, '\n');
if (eol)
{
len = eol - text;
eol++;
}
else
{
len = strlen(text);
do_text_output_multiline(TupOutputState *tstate, char *text)
{
Datum values[1];
bool isnull[1] = {false};
while (text && *text) //if the text is NULL the return, do nothing.
{
char *eol;
int len;
eol = strchr(text, '\n');
if (eol)
{
len = eol - text;
eol++;
}
else
{
len = strlen(text);
if (eol) //to check whether the eol is NULL or not.
eol += len;
}
values[0] = PointerGetDatum(cstring_to_text_with_len(text, len));
do_tup_output(tstate, values, isnull);
pfree(DatumGetPointer(values[0]));
text = eol;
}
}
}
values[0] = PointerGetDatum(cstring_to_text_with_len(text, len));
do_tup_output(tstate, values, isnull);
pfree(DatumGetPointer(values[0]));
text = eol;
}
}
Best Regards.
RingsC.
On 20 May 2016 at 19:13, Hao Lee <mixtrue@gmail.com> wrote:
>
> Hi all,
>
> Today, I am do some works on adding some customized featues to PostgreSQL 9.6 beta1. But, when i do some output to
psqlusing the fuction "do_text_output_multiline" with the string just like mentioned in mail tilte, such as "this is
a\ntestfor\nnew blank.". the PostgreSQL may lead to corruption in this function, and i debugged it that found this
functioncan not dealt with the boundaries properly. The original function code as :
>
> do_text_output_multiline(TupOutputState *tstate, char *text)
> {
> Datum values[1];
> bool isnull[1] = {false};
>
> while (*text)
> {
> char *eol;
> int len;
>
> eol = strchr(text, '\n');
> if (eol)
> {
> len = eol - text;
>
> eol++;
> }
> else
> {
> len = strlen(text);
> eol += len;
> }
>
> values[0] = PointerGetDatum(cstring_to_text_with_len(text, len));
> do_tup_output(tstate, values, isnull);
> pfree(DatumGetPointer(values[0]));
> text = eol;
> }
> }
>
Thanks for reporting this. It does seem pretty broken. I guess we've
only gotten away with this due to EXPLAIN output lines always having a
\n at the end of them, but we should fix this.
Your proposed fix looks a little bit confused. You could have just
removed the eol += len; as testing if (eol) in the else will never be
true as that else is only being hit because eol is NULL.
I shuffled things around in there a bit and came up with the attached fix.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Вложения
David Rowley <david.rowley@2ndquadrant.com> writes:
> On 20 May 2016 at 19:13, Hao Lee <mixtrue@gmail.com> wrote:
>> Today, I am do some works on adding some customized featues to PostgreSQL 9.6 beta1. But, when i do some output to
psqlusing the fuction "do_text_output_multiline" with the string just like mentioned in mail tilte, such as "this is
a\ntestfor\nnew blank.". the PostgreSQL may lead to corruption in this function, and i debugged it that found this
functioncan not dealt with the boundaries properly. The original function code as :
> Thanks for reporting this. It does seem pretty broken. I guess we've
> only gotten away with this due to EXPLAIN output lines always having a
> \n at the end of them, but we should fix this.
Agreed.
> Your proposed fix looks a little bit confused. You could have just
> removed the eol += len; as testing if (eol) in the else will never be
> true as that else is only being hit because eol is NULL.
I think really the right fix is "eol = text + len" rather than modifying
the loop condition. Almost certainly, that is what the original coder
intended, but typo'd the statement and nobody ever noticed.
> I shuffled things around in there a bit and came up with the attached fix.
I didn't like this version because it duplicated the string-conversion
code, which admittedly is only one line, but not a very simple line.
I pushed something based on "eol = text + len" instead.
regards, tom lane