[PATCH] pg_escape_literal/pg_escape_identifier for pgsql

14 years ago by Yasuo Ohgaki — view source

unread

Hi all

Current PostgreSQL has new escape functions for literals and identifiers.
Since there is no function for that, I made patch for that. Attached patch is
for trunk and tested with PostgreSQL 8.4 and 9.0.

This patch would also applied to 5.4 branch but how about 5.3?
It's pain not to have escape functions.

Any comments?

--
Yasuo Ohgaki
yohgaki@ohgaki.net

14 years ago by Hannes Magnusson — view source

unread

Hi all

Current PostgreSQL has new escape functions for literals and identifiers.
Since there is no function for that, I made patch for that. Attached patch is
for trunk and tested with PostgreSQL 8.4 and 9.0.

This patch would also applied to 5.4 branch but how about 5.3?
It's pain not to have escape functions.

Any comments?

This won't compile if HAVE_PQESCAPE isn't declared since you only
register the arginfo when it is, but use it even if it wasn't.

There is also very inconsistent usage of space vs tab in the patch.

if (!tmp) {

  php_error_docref(NULL TSRMLS_CC, E_WARNING,"Failed to escape");

```
   RETURN_FALSE;
```
}

Won't this also trigger the error if there was no from passed?
I suppose checking from_len wouldn't be stupid.

Should we really provide fallback if PQescape(Literal|Identifier)
isn't available? Did you copy the escaping from postgresql directly?

-Hannes

14 years ago by Yasuo Ohgaki — view source

unread

2011/5/20 Hannes Magnusson hannes.magnusson@gmail.com:

Hi all

Current PostgreSQL has new escape functions for literals and identifiers.
Since there is no function for that, I made patch for that. Attached patch is
for trunk and tested with PostgreSQL 8.4 and 9.0.

This patch would also applied to 5.4 branch but how about 5.3?
It's pain not to have escape functions.

Any comments?

This won't compile if HAVE_PQESCAPE isn't declared since you only
register the arginfo when it is, but use it even if it wasn't.

I suppose it would be ancient PostgreSQL (6.5?) which would not have PQescape,
I'll fix it anyway.

There is also very inconsistent usage of space vs tab in the patch.

For some reason, my emacs ignored the marker(?)
I'll fix it

if (!tmp) {

php_error_docref(NULL TSRMLS_CC, E_WARNING,"Failed to escape");

RETURN_FALSE;

}

Won't this also trigger the error if there was no from passed?
I suppose checking from_len wouldn't be stupid.

This error occurs only if libpq could not allocate memory.
Empty string should be valid string for inserting db.

Should we really provide fallback if PQescape(Literal|Identifier)
isn't available? Did you copy the escaping from postgresql directly?

Wouldn't it nice to have the same escaping capability for 8.x?
It's not possible to copy the code, since it's binded to PostgreSQL's
multibyte function. It's possible to use mbstring function for thatbut it's
not default.

I also looked into PostgreSQL code to see if it's possible to bundle
libpq, but it's closely related to core and there is no easy way to
cut down.

Most code was copied from PostgreSQL and it works like original
function. As I wrote in comment, current implementation has limitation
for some multibyte encodings. However, it works for encoding that is
compatible to ISO-8859-1. Therefore, it works for almost all encodings.

Thanks for the comment.
Is there any comment?

--
Yasuo Ohgaki

14 years ago by Yasuo Ohgaki — view source

unread

2011/5/24 Yasuo Ohgaki yohgaki@ohgaki.net:

Should we really provide fallback if PQescape(Literal|Identifier)
isn't available? Did you copy the escaping from postgresql directly?

Wouldn't it nice to have the same escaping capability for 8.x?
It's not possible to copy the code, since it's binded to PostgreSQL's
multibyte function. It's possible to use mbstring function for thatbut it's
not default.

I forgot to mention.

It may seems to strange way to escape strings, (i.e. ' E' prefix) but all
currently supported PostgreSQL versions accept this escape.

--
Yasuo Ohgaki
yohgaki@ohgaki.net

14 years ago by Hannes Magnusson — view source

unread

2011/5/20 Hannes Magnusson hannes.magnusson@gmail.com:

Hi all

Current PostgreSQL has new escape functions for literals and identifiers.
Since there is no function for that, I made patch for that. Attached patch is
for trunk and tested with PostgreSQL 8.4 and 9.0.

This patch would also applied to 5.4 branch but how about 5.3?
It's pain not to have escape functions.

Any comments?

This won't compile if HAVE_PQESCAPE isn't declared since you only
register the arginfo when it is, but use it even if it wasn't.

I suppose it would be ancient PostgreSQL (6.5?) which would not have PQescape,
I'll fix it anyway.

There is also very inconsistent usage of space vs tab in the patch.

For some reason, my emacs ignored the marker(?)
I'll fix it

if (!tmp) {

php_error_docref(NULL TSRMLS_CC, E_WARNING,"Failed to escape");

RETURN_FALSE;

}

Won't this also trigger the error if there was no from passed?
I suppose checking from_len wouldn't be stupid.

This error occurs only if libpq could not allocate memory.
Empty string should be valid string for inserting db.

So if I pass in an empty string, libpq will throw some escape
characters around it?

Should we really provide fallback if PQescape(Literal|Identifier)
isn't available? Did you copy the escaping from postgresql directly?

Wouldn't it nice to have the same escaping capability for 8.x?
It's not possible to copy the code, since it's binded to PostgreSQL's
multibyte function. It's possible to use mbstring function for thatbut it's
not default.

I'm just worried about if there is a logic error there then we are
creating security issues while pretending that the escaping is
perfectly acceptable and working.

Thanks for the comment.
Is there any comment?

Go for it

-Hannes