Neil Conway wrote:
> I think we _can_ do it that way, it's just a question of whether that is
> the best approach. I think the solution I outlined before would work
> fine: pass the length of the working buffer to ParseDateTime(), and
> reject the input only if the parsing process actually requires more
> working space than was provided. [...]

> Or we could rewrite how we do parsing, which seems to be what you're
> suggesting.

Any thoughts on this, Tom? If you're planning on doing the rewrite you
described, that's fine; alternatively I'm happy to implement the fix
that I described above.

-Neil

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Neil Conway <neilc@samurai.com> writes:
> Any thoughts on this, Tom? If you're planning on doing the rewrite you
> described, that's fine; alternatively I'm happy to implement the fix
> that I described above.

I'm not set on it --- fix it as you suggested.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

Tom Lane wrote:
> I'm not set on it --- fix it as you suggested.

Attached is a patch that implements this. I'm not especially happy about
the implementation: defining _two_ local macros (that both
doubly-evaluate one of their arguments) is pretty ugly, but I didn't see
a cleaner alternative -- suggestions welcome.

This patch fixes the stack overrun. Since we now pass the length of the
working space into ParseDateTime(), it is also easy to fix Mark's bug: I
just increased the size of the interval_in() working buffer to 256
bytes. I didn't touch any of the other input buffer sizes, although that
shouldn't be taken as a vote of confidence that they are correct At
least the consequence of an insufficiently-large buffer is limited to
rejecting otherwise valid input, rather than potentially scribbling on
the stack.

-Neil


---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

Neil Conway <neilc@samurai.com> writes:
> Attached is a patch that implements this. I'm not especially happy about
> the implementation: defining _two_ local macros (that both
> doubly-evaluate one of their arguments) is pretty ugly, but I didn't see
> a cleaner alternative -- suggestions welcome.

Considering that you're incrementing bufptr inside the macro, it hardly
seems that double-evaluation is a problem: the argument pretty much has
to be a variable. OTOH there is no reason for the input argument to be
treated that way. I'd suggest just one macro

#define APPEND_CHAR(bufptr, end, newchar) \
do \
{ \
if (((bufptr) + 1) >= (end)) \
return DTERR_BAD_FORMAT; \
*(bufptr)++ = (newchar); \
} while (0);

used as

APPEND_CHAR(bufp, bufend, *cp++);
APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));

respectively.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

I wrote:
> #define APPEND_CHAR(bufptr, end, newchar) \
> do \
> { \
> if (((bufptr) + 1) >= (end)) \
> return DTERR_BAD_FORMAT; \
> *(bufptr)++ = (newchar); \
> } while (0);

Oh BTW, there definitely should not be a semicolon at the end
of that macro ...

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Tom Lane wrote:
> Considering that you're incrementing bufptr inside the macro, it hardly
> seems that double-evaluation is a problem: the argument pretty much has
> to be a variable. OTOH there is no reason for the input argument to be
> treated that way. I'd suggest just one macro [...]

Ah, yeah, that works. Attached is a revised patch -- I'll apply it
tomorrow barring any objections. I also fixed the semi-colon -- thanks
for the review.

-Neil


---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

Neil Conway wrote:
> Ah, yeah, that works. Attached is a revised patch -- I'll apply it
> tomorrow barring any objections. I also fixed the semi-colon -- thanks
> for the review.

Patch applied to HEAD, REL8_0_STABLE, and REL7_4_STABLE. Thanks for the
report, Mark.

-Neil

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)