Thread: [HACKERS] Try to fix endless loop in ecpg with informix mode
Hi,
I tried some tests with ecpg informix mode.The reason is:
In informix mode, ecpg can accept
string form of float number when processing query result.
During checking the string form of float number, it seems
that ecpg forgot to skip characters after '.'.
Then outer loop will never stop because it hopes to see '\0'.
Attachment
On Wed, Nov 1, 2017 at 12:22 PM, 高增琦 <pgf00a@gmail.com> wrote: > Any comments? > Hi, You should register these patches for the next commitfest at https://commitfest.postgresql.org/15/. As Michael pointed out earlier, this commitfest will start soon so you should add your patches quickly. Regards. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
> Any comments? Sorry, I've been working through the backlog of three weeks of traveling. > > I tried some tests with ecpg informix mode. > > When trying to store float data into a integer var, I got endless > > loop. > > > > The reason is: > > In informix mode, ecpg can accept > > string form of float number when processing query result. > > During checking the string form of float number, it seems > > that ecpg forgot to skip characters after '.'. > > Then outer loop will never stop because it hopes to see '\0'. > > > > The first patch will reproduce the problem in ecpg's regress test. > > The second patch tries to fix it in simple way. Thanks for spotting and fixing. I changed your patch slightly and made it check if the rest of the data is indeed digits, or else it would accept something like "7.hello" as "7". Committed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Thanks for commit.
I am afraid the changes may separate "7.a" to "7" and "a", then error out```
- } while (**scan_length != ' ' && **scan_length != '\0');
- return false;
+ } while (isdigit(**scan_length));
```
2017-11-01 20:35 GMT+08:00 Michael Meskes <meskes@postgresql.org>:
> Any comments?
Sorry, I've been working through the backlog of three weeks of
traveling.
> > I tried some tests with ecpg informix mode.
> > When trying to store float data into a integer var, I got endless
> > loop.
> >
> > The reason is:
> > In informix mode, ecpg can accept
> > string form of float number when processing query result.
> > During checking the string form of float number, it seems
> > that ecpg forgot to skip characters after '.'.
> > Then outer loop will never stop because it hopes to see '\0'.
> >
> > The first patch will reproduce the problem in ecpg's regress test.
> > The second patch tries to fix it in simple way.
Thanks for spotting and fixing. I changed your patch slightly and made
it check if the rest of the data is indeed digits, or else it would
accept something like "7.hello" as "7".
Committed.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
--
Diff from the head:
(use the following if to decide true or false)
```
diff --git a/src/interfaces/ecpg/ecpglib/data.c b/src/interfaces/ecpg/ecpglib/data.c
index 5375934..1621e7b 100644
--- a/src/interfaces/ecpg/ecpglib/data.c
+++ b/src/interfaces/ecpg/ecpglib/data.c
@@ -57,8 +57,7 @@ garbage_left(enum ARRAY_TYPE isarray, char **scan_length, enum COMPAT_MODE compa
/* skip invalid characters */
do {
(*scan_length)++;
- } while (**scan_length != ' ' && **scan_length != '\0' && isdigit(**scan_length));
- return false;
+ } while (isdigit(**scan_length));
}
if (**scan_length != ' ' && **scan_length != '\0')
```
(use the following if to decide true or false)
```
diff --git a/src/interfaces/ecpg/ecpglib/data.c b/src/interfaces/ecpg/ecpglib/data.c
index 5375934..1621e7b 100644
--- a/src/interfaces/ecpg/ecpglib/data.c
+++ b/src/interfaces/ecpg/ecpglib/data.c
@@ -57,8 +57,7 @@ garbage_left(enum ARRAY_TYPE isarray, char **scan_length, enum COMPAT_MODE compa
/* skip invalid characters */
do {
(*scan_length)++;
- } while (**scan_length != ' ' && **scan_length != '\0' && isdigit(**scan_length));
- return false;
+ } while (isdigit(**scan_length));
}
if (**scan_length != ' ' && **scan_length != '\0')
```
2017-11-02 11:07 GMT+08:00 高增琦 <pgf00a@gmail.com>:
How about changes as below? (use following the if to decide true or false)with "invalid input syntax for type int" for "a".Thanks for commit.I am afraid the changes may separate "7.a" to "7" and "a", then error out
```
- } while (**scan_length != ' ' && **scan_length != '\0');
- return false;
+ } while (isdigit(**scan_length));
```--2017-11-01 20:35 GMT+08:00 Michael Meskes <meskes@postgresql.org>:> Any comments?
Sorry, I've been working through the backlog of three weeks of
traveling.
> > I tried some tests with ecpg informix mode.
> > When trying to store float data into a integer var, I got endless
> > loop.
> >
> > The reason is:
> > In informix mode, ecpg can accept
> > string form of float number when processing query result.
> > During checking the string form of float number, it seems
> > that ecpg forgot to skip characters after '.'.
> > Then outer loop will never stop because it hopes to see '\0'.
> >
> > The first patch will reproduce the problem in ecpg's regress test.
> > The second patch tries to fix it in simple way.
Thanks for spotting and fixing. I changed your patch slightly and made
it check if the rest of the data is indeed digits, or else it would
accept something like "7.hello" as "7".
Committed.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
--
> I am afraid the changes may separate "7.a" to "7" and "a", then error > out > with "invalid input syntax for type int" for "a". Which is correct, is it not? > How about changes as below? (use following the if to decide true or > false) > ... > return false; > + } while (isdigit(**scan_length)); Yes, this is certainly correct and better than what I committed. What was I thinking yesterday? I think the same function is used for identifying garbage in floats which might ask for different logic. Let me check. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
I found the last commit changed as:```
/* skip invalid characters */
do {
(*scan_length)++;
- } while (**scan_length != ' ' && **scan_length != '\0' && isdigit(**scan_length));
+ } while (isdigit(**scan_length));
return false;
}
```
It will still return false if we got non-digital characters after ".",
then it will error out "invalid input syntax for type int" for "a" . (if input is "7.a")
Although this error message is not wrong, I think it should be better to
give error message as "invalid input syntax for type int" for "7.a".
This could be done by delete "return false;" after "while(...)", let
the following if to decide which to return.
2017-11-02 15:25 GMT+08:00 Michael Meskes <meskes@postgresql.org>:
> I am afraid the changes may separate "7.a" to "7" and "a", then error
> out
> with "invalid input syntax for type int" for "a".
Which is correct, is it not?
> How about changes as below? (use following the if to decide true or
> false)
> ...
> return false;
> + } while (isdigit(**scan_length));
Yes, this is certainly correct and better than what I committed. What
was I thinking yesterday?
I think the same function is used for identifying garbage in floats
which might ask for different logic. Let me check.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
--
> Although this error message is not wrong, I think it should be better > to > give error message as "invalid input syntax for type int" for "7.a". > This could be done by delete "return false;" after "while(...)", let > the following if to decide which to return. Ah, now I understand. Sorry, I completely misunderstood your prior email. Yes, you're right, I will change the code accordingly. Thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers