Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:52512
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.170 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :cc:content-type:content-transfer-encoding;
        b=ordkcu/o2HJhByqoBhCZClUPdw/3XurbehrvSfPYX+p+SCnHITQr4OUzIGESqs9WwU
         BV7VrwLoIoQFE1vrcy2Cknr0QIEn4HXdutwi7dgXkfI1Fjfa1rHZX2358jCqK1q9j8S9
         OjVSTcRk8pF6wNqMYHIzSeF0bIIfg4+zVsfbM=
MIME-Version: 1.0
In-Reply-To: <BANLkTikZF6gSWFD889rYWtzDLzuhvxnPTQ@mail.gmail.com>
References: <BANLkTik4GVWKdRxiZs+--tNdTsSD1QBNnA@mail.gmail.com>
	<BANLkTimoP95r6PgtCrgByBvLOvriu-pdAw@mail.gmail.com>
	<BANLkTikZF6gSWFD889rYWtzDLzuhvxnPTQ@mail.gmail.com>
Date: Tue, 24 May 2011 09:28:54 +0200
Message-ID: <BANLkTi=cng5TbvdM5sRkhFYBwfF7rVJihw@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [PATCH] pg_escape_literal/pg_escape_identifier for pgsql
From: hannes.magnusson@gmail.com (Hannes Magnusson)

On Tue, May 24, 2011 at 06:37, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> 2011/5/20 Hannes Magnusson <hannes.magnusson@gmail.com>:
>> On Fri, May 20, 2011 at 03:24, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>>> Hi all
>>>
>>> Current PostgreSQL has new escape functions for literals and identifier=
s.
>>> Since there is no function for that, I made patch for that. Attached pa=
tch 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 PQes=
cape,
> 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
>
>>
>> + =C2=A0 =C2=A0if (!tmp) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 php_error_docref(NULL=
 TSRMLS_CC, E_WARNING,"Failed to escape");
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0RETURN_FALSE;
>> + =C2=A0 =C2=A0}
>>
>> 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