PHP 5.2.7RC3 Reminder

16 years ago by Ilia Alshanetsky — view source

unread

Just a quick reminder for all of the developers, RC3 (the final RC)
will be tagged tomorrow afternoon (EST) time, please try to get
whatever patches (bug fixes only, please) you still want into it. If
there are reasons to delay, critical patches missing, please let me
know.

Thanks,

Ilia Alshanetsky
5.2 Release Master

16 years ago by Jani Taskinen — view source

unread

Ilia Alshanetsky wrote:

Just a quick reminder for all of the developers, RC3 (the final RC) will
be tagged tomorrow afternoon (EST) time, please try to get whatever
patches (bug fixes only, please) you still want into it. If there are
reasons to delay, critical patches missing, please let me know.

http://bugs.php.net/bug.php?id=44938&edit=1

Marked critical but propably isn't. Also depatable whether it's a PHP
bug at all..

--Jani

16 years ago by Ilia Alshanetsky — view source

unread

The problem here is that the domain parameter will eventually make its
way into the file path and result in an overflow of the MAX_PATHLEN
there (I suspect). The problem however (IMHO) lies in gettext that
does not check the file path length before using it. For us to do the
check will require an arbitrary limit, since the domain comprises only
of a portion of the path and just putting a limit on its length of
MAX_PATHLEN would not be enough.

Ilia Alshanetsky wrote:

Just a quick reminder for all of the developers, RC3 (the final RC)
will be tagged tomorrow afternoon (EST) time, please try to get
whatever patches (bug fixes only, please) you still want into it.
If there are reasons to delay, critical patches missing, please let
me know.

http://bugs.php.net/bug.php?id=44938&edit=1

Marked critical but propably isn't. Also depatable whether it's a
PHP bug at all..

--Jani

Ilia Alshanetsky

16 years ago by Christian Schneider — view source

unread

Jani Taskinen wrote:

http://bugs.php.net/bug.php?id=44938&edit=1

Marked critical but propably isn't. Also depatable whether it's a PHP
bug at all..

Probably the library exhausting memory using alloca I'd say.

I whipped together a little patch against HEAD which restricts the
length of a text domain string to 10000 bytes to avoid problems in the
underlying library.

Note: I haven't been able to compile HEAD right now so I couldn't test
the patch really. But it compiles and should be rather trivial to
review/test.

Chris

16 years ago by Ilia Alshanetsky — view source

unread

Not a bad idea, although I'd put a limit of the domain length o 4kb. I
can't see someone needing a domain longer then that, heck even 1kb
should be more then enough.

Jani Taskinen wrote:

http://bugs.php.net/bug.php?id=44938&edit=1

Marked critical but propably isn't. Also depatable whether it's a PHP
bug at all..

Probably the library exhausting memory using alloca I'd say.

I whipped together a little patch against HEAD which restricts the
length of a text domain string to 10000 bytes to avoid problems in the
underlying library.

Note: I haven't been able to compile HEAD right now so I couldn't test
the patch really. But it compiles and should be rather trivial to
review/test.

Chris
Index: ext/gettext/gettext.c
===================================================================
RCS file: /repository/php-src/ext/gettext/gettext.c,v
retrieving revision 1.58
diff -u -r1.58 gettext.c
--- ext/gettext/gettext.c 24 Oct 2008 14:34:13 -0000 1.58
+++ ext/gettext/gettext.c 29 Oct 2008 13:47:15 -0000
@@ -30,6 +30,8 @@
#include "ext/standard/info.h"
#include "php_gettext.h"

+#define MAX_DOMAIN_LENGTH 10000 /* Maximum length of textdomain
name length */

/* {{{ arginfo */
ZEND_BEGIN_ARG_INFO(arginfo_textdomain, 0)
ZEND_ARG_INFO(0, domain)
@@ -162,6 +164,10 @@
return;
}
if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
if (!domain_len || (domain_len == 1 && *domain_str == '0')) {
domain_str = NULL;
}
@@ -193,6 +199,10 @@
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,
"s&s&", &domain_str, &domain_len,
ZEND_U_CONVERTER(UG(filesystem_encoding_conv)), &msgid_str,
&msgid_len, UG(ascii_conv))) {
return;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
RETURN_STRING(dgettext(domain_str, msgid_str), ZSTR_DUPLICATE);
}
/* }}} */
@@ -208,6 +218,10 @@
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,
"s&s&l", &domain_str, &domain_len,
ZEND_U_CONVERTER(UG(filesystem_encoding_conv)), &msgid_str,
&msgid_len, UG(ascii_conv), &category)) {
return;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
RETURN_STRING(dcgettext(domain_str, msgid_str, category),
ZSTR_DUPLICATE);
}
/* }}} */
@@ -223,6 +237,10 @@
return;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long");
RETURN_FALSE;
}
if (!domain_len) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "the first parameter
must not be empty");
RETURN_FALSE;
@@ -273,6 +291,10 @@
RETURN_FALSE;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
if ((msgstr = dngettext(domain_str, msgid_str1, msgid_str2,
count))) {
RETURN_STRING(msgstr, ZSTR_DUPLICATE);
} else {
@@ -295,6 +317,10 @@
RETURN_FALSE;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
if ((msgstr = dcngettext(domain_str, msgid_str1, msgid_str2, count,
category))) {
RETURN_STRING(msgstr, ZSTR_DUPLICATE);
} else {
@@ -316,6 +342,10 @@
return;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long");
RETURN_FALSE;
}
if (!codeset_len) {
codeset_str = NULL;
}
--

Ilia Alshanetsky

16 years ago by Ilia Alshanetsky — view source

unread

Christian,

Your patch with slight revisions just went in.

Jani Taskinen wrote:

http://bugs.php.net/bug.php?id=44938&edit=1

Marked critical but propably isn't. Also depatable whether it's a PHP
bug at all..

Probably the library exhausting memory using alloca I'd say.

I whipped together a little patch against HEAD which restricts the
length of a text domain string to 10000 bytes to avoid problems in the
underlying library.

Note: I haven't been able to compile HEAD right now so I couldn't test
the patch really. But it compiles and should be rather trivial to
review/test.

Chris
Index: ext/gettext/gettext.c
===================================================================
RCS file: /repository/php-src/ext/gettext/gettext.c,v
retrieving revision 1.58
diff -u -r1.58 gettext.c
--- ext/gettext/gettext.c 24 Oct 2008 14:34:13 -0000 1.58
+++ ext/gettext/gettext.c 29 Oct 2008 13:47:15 -0000
@@ -30,6 +30,8 @@
#include "ext/standard/info.h"
#include "php_gettext.h"

+#define MAX_DOMAIN_LENGTH 10000 /* Maximum length of textdomain
name length */

/* {{{ arginfo */
ZEND_BEGIN_ARG_INFO(arginfo_textdomain, 0)
ZEND_ARG_INFO(0, domain)
@@ -162,6 +164,10 @@
return;
}
if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
if (!domain_len || (domain_len == 1 && *domain_str == '0')) {
domain_str = NULL;
}
@@ -193,6 +199,10 @@
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,
"s&s&", &domain_str, &domain_len,
ZEND_U_CONVERTER(UG(filesystem_encoding_conv)), &msgid_str,
&msgid_len, UG(ascii_conv))) {
return;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
RETURN_STRING(dgettext(domain_str, msgid_str), ZSTR_DUPLICATE);
}
/* }}} */
@@ -208,6 +218,10 @@
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,
"s&s&l", &domain_str, &domain_len,
ZEND_U_CONVERTER(UG(filesystem_encoding_conv)), &msgid_str,
&msgid_len, UG(ascii_conv), &category)) {
return;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
RETURN_STRING(dcgettext(domain_str, msgid_str, category),
ZSTR_DUPLICATE);
}
/* }}} */
@@ -223,6 +237,10 @@
return;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long");
RETURN_FALSE;
}
if (!domain_len) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "the first parameter
must not be empty");
RETURN_FALSE;
@@ -273,6 +291,10 @@
RETURN_FALSE;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
if ((msgstr = dngettext(domain_str, msgid_str1, msgid_str2,
count))) {
RETURN_STRING(msgstr, ZSTR_DUPLICATE);
} else {
@@ -295,6 +317,10 @@
RETURN_FALSE;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long, ignoring");
domain_str = NULL;
}
if ((msgstr = dcngettext(domain_str, msgid_str1, msgid_str2, count,
category))) {
RETURN_STRING(msgstr, ZSTR_DUPLICATE);
} else {
@@ -316,6 +342,10 @@
return;
}

if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed too  
long");
RETURN_FALSE;
}
if (!codeset_len) {
codeset_str = NULL;
}
--

Ilia Alshanetsky

16 years ago by Lars Strojny — view source

unread

Hi Illia,

Am Mittwoch, den 29.10.2008, 17:02 -0400 schrieb Ilia Alshanetsky:
[...]

Your patch with slight revisions just went in.

The sad thing is, that the patch just fixes half of the issue. If your
try the examples in the bugreport, most of the still work. We have also
check the msgids for overflows. I'm currently working on a patch
including tests.

cu, Lars

16 years ago by Pierre Joye — view source

unread

Just a quick reminder for all of the developers, RC3 (the final RC)
will be tagged tomorrow afternoon (EST) time, please try to get
whatever patches (bug fixes only, please) you still want into it. If
there are reasons to delay, critical patches missing, please let me
know.

Can we do on Tuesday next week instead.?

Many of us are in the IPC conference these days and won't be back in
normal activities until Monday. Doing so will also let me be sure that
everything works on windows before you tag this release.

Cheers,

Pierre

http://blog.thepimp.net | http://www.libgd.org