Re: [PHP-DEV] default charset confusion
On Tue, Mar 13, 2012 at 1:52 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote:
2012/3/13 Rasmus Lerdorf ras...@lerdorf.com:
On 03/12/2012 03:05 AM, Yasuo Ohgaki wrote:
I thought default_charset became UTF-8, so I was expecting
following HTTP header.
content-type text/html; charset=UTF-8
However, I got empty charset (missing 'charset=UTF-8').
So I looked up to source and found the line in SAPI.h
293 #define SAPI_DEFAULT_CHARSET
Empty string should be UTF-8, isn't it?
No, we can't force an output charset on people since it would end up
breaking a lot of sites.
Right, so may be for the next major release? 5.5.0?
As the first XSS advisory in 2000 states, explicitly setting char coding
will
prevent certain XSS. Recent browsers have much better encoding handing,
but setting encoding explicitly is better for security still.
PHP 5.3's determine_charset behaves exactly like 5.4's. In 5.3 we have:
if (charset_hint == NULL)
return cs_8859_1;
and in 5.4 we have:
if (charset_hint == NULL)
return cs_utf_8;
So there is no difference in their guessing when there is no hint, the
only difference is that in 5.4 we choose utf8 and in 5.3 we choose
8859-1 in that case.
I got this with 5.3
?php
echo htmlentities('日本語UTF-8',ENT_QUOTES);
echo htmlentities('日本語UTF-8',ENT_QUOTES, 'UTF-8');
lt;aelig;�yen;aelig;�not;egrave;ordf;�UTF8
gt;lt;日本語UTF-8gt;
So people migrating from 5.3 to 5.4 should not have problems.
Migration older than 5.3 to 5.4 will be problematic.
I always set all parameters for htmlentities/htmlspecialchars, therefore
I haven't noticed this was changed from 5.3. They may be migrating from
5.2 or older. (RHEL5 uses 5.1)
Since PHP does not have default multibyte module, it may be good for having
input_encoding
internal_encoding
output_encoding
I would then propose to make mbstring compile time mandatory.
I'm against yet another global ini setting, I find the actual ini settings
confusing enough to add one more that would moreover reflect mbstring one's
(and add more and more confusion).
Why not turn ext/mbstring mandatory at compile time, for all future PHP
versions, like preg or spl are ?
We do need multibyte handling either. ZendEngine takes advantage of
mbstring for internal encoding as well, so I probably missed something as
why it is still possible to --disable-mbstring (or not add
--enable-mbstring) when compiling ? Has it a huge performance impact ?
Thank you :)
Julien.P
Re: [PHP-DEV] default charset confusion
Correct me if I'm wrong, but I believe Zend Multibyte is now enabled by default in PHP 5.4. - Mike On Wed, Mar 14, 2012 at 9:24 AM, Ferenc Kovacs tyr...@gmail.com wrote: I would then propose to make mbstring compile time mandatory. I'm against yet another global ini setting, I find the actual ini settings confusing enough to add one more that would moreover reflect mbstring one's (and add more and more confusion). Why not turn ext/mbstring mandatory at compile time, for all future PHP versions, like preg or spl are ? We do need multibyte handling either. ZendEngine takes advantage of mbstring for internal encoding as well, so I probably missed something as why it is still possible to --disable-mbstring (or not add --enable-mbstring) when compiling ? Has it a huge performance impact ? Thank you :) Julien.P see http://www.mail-archive.com/internals@lists.php.net/msg48452.html http://lxr.php.net/opengrok/xref/PHP_5_4/UPGRADING#91 and http://www.mail-archive.com/internals@lists.php.net/msg53863.html basically the mbstring code in the ZE is only used if you enable zend.multibyte, which is disabled by default, so it isn't mandatory to have ext/mbstring for the default build/setup. as you can see from the last link, I would support having ext/mbstring builtin and always enabled, but I would like to hear from more people about the pros and cons. -- Ferenc Kovács @Tyr43l - http://tyrael.hu -- --- My command is this: Love each other as I have loved you. John 15:12 ---
Re: [PHP-DEV] default charset confusion
On Wed, Mar 14, 2012 at 3:29 PM, Michael Stowe m...@mikestowe.com wrote: Correct me if I'm wrong, but I believe Zend Multibyte is now enabled by default in PHP 5.4. - Mike http://lxr.php.net/opengrok/xref/PHP_5_4/UPGRADING#91 http://lxr.php.net/opengrok/xref/PHP_5_4/Zend/zend.c#108 http://lxr.php.net/opengrok/xref/PHP_5_4/php.ini-development#358 http://lxr.php.net/opengrok/xref/PHP_5_4/php.ini-production#358 we just moved the switch from compilation time to runtime, so the code is there, if you want to enable it, you don't have to recompile php but only have to change an ini setting, but it isn't turned on by default. AFAIK -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] default charset confusion
On Wed, 14 Mar 2012 14:55:17 +0100, jpauli jpa...@php.net wrote: I would then propose to make mbstring compile time mandatory. I'm completely against these kind of lazy solutions. Yes, let's add strong coupling (already starting to smell) to one of the largest extensions and make it compile time mandatory because it simplifies the implementation of a dubiously useful feature like Zend multibyte. Remember PHP is sometimes used in environments with limited memory/disk space. Also mbstring takes a long time to build (relatively speaking). Just that would be a strong argument against making it mandatory, at least for people like me that compile PHP with --disable-all very frequently. I'm against yet another global ini setting, I find the actual ini settings confusing enough to add one more that would moreover reflect mbstring one's (and add more and more confusion). Why not turn ext/mbstring mandatory at compile time, for all future PHP versions, like preg or spl are ? We do need multibyte handling either. ZendEngine takes advantage of mbstring for internal encoding as well, so I probably missed something as why it is still possible to --disable-mbstring (or not add --enable-mbstring) when compiling ? Has it a huge performance impact ? mbstring hooks to basically all phases of PHP process/request startup/shutdown. Some efforts were made to mitigate the impact of this in 5.4 (see e.g. r301068), but at least some impact is inevitable. Of course, if you start enabling certain features of mbstring (zend multibyte hooks, translation of input variables, function overload) then it starts to be significant. However, there are other more compelling reasons not to make it required (see above). -- Gustavo Lopes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] readline extension rl_bind_key addition and other enhancements
I was recently involved in a project that relied heavily on readline to provide
console text input capabilities. However I soon noticed that the current
readline is lacking some important functionality.
This patch applies only to PHP compiled with libreadline (and don't have an
effect if compiled with libeditline). It adds/modified the following:
- Adds support for rl_bind_key function by exposing:
function readline_bind_key_function($key, $callback_func)
where:
$key: Key code to bind to.
$callback_func: A callback function in the form:
function callback($key, $count) where $key and $count are the same parameters
passed from rl_bind_key()
Setting $callback_func to null will cause the binding to be removed for $key.
- Modifies the behavior of readline_info() to allow for setting the readline
properties point and end (it used to only read them but not set them).
Patch below:
--- php-5.3.10/ext/readline/readline.c 2012-01-01 15:15:04.0 +0200
+++ php-5.3.10-patched/ext/readline/readline.c 2012-03-13 09:35:43.0
+0200
@@ -57,9 +57,12 @@ PHP_FUNCTION(readline_callback_read_char
PHP_FUNCTION(readline_callback_handler_remove);
PHP_FUNCTION(readline_redisplay);
PHP_FUNCTION(readline_on_new_line);
+PHP_FUNCTION(readline_bind_key_function);
static zval *_prepped_callback = NULL;
+static HashTable *_bind_key_functions_ht = NULL;
+
#endif
static zval *_readline_completion = NULL;
@@ -121,6 +124,12 @@ ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_INFO(arginfo_readline_on_new_line, 0)
ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_readline_bind_key_function, 0, 0, 2)
+ ZEND_ARG_INFO(0, key)
+ ZEND_ARG_INFO(0, funcname)
+ZEND_END_ARG_INFO()
+
#endif
/* }}} */
@@ -142,6 +151,7 @@ static const zend_function_entry php_rea
PHP_FE(readline_callback_handler_remove,
arginfo_readline_callback_handler_remove)
PHP_FE(readline_redisplay, arginfo_readline_redisplay)
PHP_FE(readline_on_new_line, arginfo_readline_on_new_line)
+ PHP_FE(readline_bind_key_function, arginfo_readline_bind_key_function)
#endif
PHP_FE_END
};
@@ -165,6 +175,9 @@ ZEND_GET_MODULE(readline)
PHP_MINIT_FUNCTION(readline)
{
+ ALLOC_HASHTABLE(_bind_key_functions_ht);
+ zend_hash_init(_bind_key_functions_ht, 255, NULL, ZVAL_PTR_DTOR, 0);
+
using_history();
return SUCCESS;
}
@@ -181,6 +194,9 @@ PHP_RSHUTDOWN_FUNCTION(readline)
zval_ptr_dtor(_prepped_callback);
_prepped_callback = 0;
}
+
+ zend_hash_destroy(_bind_key_functions_ht);
+ FREE_HASHTABLE(_bind_key_functions_ht);
#endif
return SUCCESS;
@@ -254,8 +270,16 @@ PHP_FUNCTION(readline_info)
}
RETVAL_STRING(SAFE_STRING(oldstr),1);
} else if (!strcasecmp(what, point)) {
+ if (value) {
+ convert_to_long_ex(value);
+ rl_point = Z_LVAL_PP(value);
+ }
RETVAL_LONG(rl_point);
} else if (!strcasecmp(what, end)) {
+ if (value) {
+ convert_to_long_ex(value);
+ rl_end = Z_LVAL_PP(value);
+ }
RETVAL_LONG(rl_end);
#ifdef HAVE_LIBREADLINE
} else if (!strcasecmp(what, mark)) {
@@ -612,6 +636,74 @@ PHP_FUNCTION(readline_on_new_line)
}
/* }}} */
+/* {{{ proto bool readline_bind_key_function(string funcname)
+ Readline rl_bind_key */
+
+static int _readline_bind_key_cb(int count, int key)
+{
+ zval *params[2];
+ TSRMLS_FETCH();
+
+ params[0]=_readline_long_zval(key);
+ params[1]=_readline_long_zval(count);
+
+ zval **_callback_func = NULL;
+ long _key = key;
+ int r = zend_hash_find(_bind_key_functions_ht, (char *) _key,
sizeof(_key), (void **)_callback_func);
+ if (r == -1)
+ return -1;
+
+ int _return_int = 0;
+ zval *_callback_return = NULL;
+ MAKE_STD_ZVAL(_callback_return);
+ if (call_user_function(CG(function_table), NULL, *_callback_func,
_callback_return, 2, params TSRMLS_CC) == SUCCESS) {
+ _return_int = _callback_return-value.lval;
+ zval_dtor(_callback_return);
+ FREE_ZVAL(_callback_return);
+ }
+
+ return _return_int;
+}
+
+PHP_FUNCTION(readline_bind_key_function)
+{
+ long key;
+ zval *arg = NULL;
+ char *name = NULL;
+
+ if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, l!z,
key, arg)) {
+ RETURN_FALSE;
+ }
+
+ if (Z_TYPE_P(arg) == IS_NULL) {
+ zend_hash_del(_bind_key_functions_ht, (char *)key,
sizeof(key));
+ rl_bind_key(key, rl_insert);
+ }
+ else {
+ if
Re: [PHP-DEV] default charset confusion
On Wed, Mar 14, 2012 at 3:37 PM, Gustavo Lopes glo...@nebm.ist.utl.ptwrote: On Wed, 14 Mar 2012 14:55:17 +0100, jpauli jpa...@php.net wrote: I would then propose to make mbstring compile time mandatory. I'm completely against these kind of lazy solutions. Yes, let's add strong coupling (already starting to smell) to one of the largest extensions and make it compile time mandatory because it simplifies the implementation of a dubiously useful feature like Zend multibyte. Remember PHP is sometimes used in environments with limited memory/disk space. Also mbstring takes a long time to build (relatively speaking). Just that would be a strong argument against making it mandatory, at least for people like me that compile PHP with --disable-all very frequently. I'm against yet another global ini setting, I find the actual ini settings confusing enough to add one more that would moreover reflect mbstring one's (and add more and more confusion). Why not turn ext/mbstring mandatory at compile time, for all future PHP versions, like preg or spl are ? We do need multibyte handling either. ZendEngine takes advantage of mbstring for internal encoding as well, so I probably missed something as why it is still possible to --disable-mbstring (or not add --enable-mbstring) when compiling ? Has it a huge performance impact ? mbstring hooks to basically all phases of PHP process/request startup/shutdown. Some efforts were made to mitigate the impact of this in 5.4 (see e.g. r301068), but at least some impact is inevitable. Of course, if you start enabling certain features of mbstring (zend multibyte hooks, translation of input variables, function overload) then it starts to be significant. However, there are other more compelling reasons not to make it required (see above). -- Gustavo Lopes That makes sense to me :-) But we should think about complexity in the final choice. Having something like internal_encoding adding in PHP.ini will confuse people, at least, if we dont clearly explain them what the setting is for. The name is nearly the same as mbstring's. I recently opened a doc bug about multibyte handling in 5.4 (#61373) , as the documentation is really light on that point Julien.P
Re: [PHP-DEV] set the PHP_INI_ENTRY_* values the same as for php.ini-production
On Mon, Jul 25, 2011 at 12:34 PM, Richard Quadling rquadl...@gmail.comwrote: On 23 July 2011 23:29, Ferenc Kovacs tyr...@gmail.com wrote: hi. We had a discussion about the magic_quotes removal that it is weird that even though that mq was deprecated in 5.3, we still have/had that enabled by default (if you didn't set it from the command line or through a php.ini, the default value which is set from the source by the PHP_INI_ENTRY_* macros). I would propose that the defaul values(PHP_INI_ENTRY_*) and the php.ini-production should be keep in sync as much as possible. for one thing, this would be less confusing (what do we mean by default? PHP_INI_ENTRY_* or the php.ini? why are they different? etc.), the second thing would be the principle of the fail-secure principle: usually the production settings are more secure and less verbose(display_errors for example). what do you think? of course, if this keeps traction, a proper RFC would be needed, but for now, I'm just throwing ideas. My preference would be to have the defaults be for a production environment. So, if you do absolutely nothing except copy php.exe and a few DLLs, (no ini file), the defaults work to an agreed safe standard. I would envisage that this standard changes over time if weaknesses are found and exploited (I'm not saying there are any). I would then like 1 ini file that only contains specific changes to the defaults for a development environment. I would also like 1 ini file that exactly matches the defaults and that this file must be maintained in line with the internal macros. It would serve as a one stop place to see all the settings as they are defined by the PHP group and could indicate the preference for production and development. So, for a truly lazy developer, it is 1 ini file rename (activate the development environment) and only the agreed entries are amended from the production safe environment, rather than overriding any defaults from the macros, which could have changed and are now out of sync with the INI file. Of course, what is considered safe is for you guys to decide, but with so many settings in the INI files and, as we have seen, disagreement between the internal macros, a little consensus should go a long way to be able to help ISPs and shared hosters have a more uniform standard. Maybe, and this is right of the top of my head, if PHP is installed for a production environment with no INI file, or if an ini file doesn't alter any of the core settings (maybe a separation of INI files for core and extensions?), it could be labelled/considered as a virgin PHP install. Something which could be marketed / advertised. Essentially, the PHP Group agree that for a production environment, these are the settings that are the safest to use. If there are considerations that need to be made for shared hosters, then maybe some mechanism to set these appropriately. So, for a user coming to an ISP and looking at hosting, they can see We use Virgin PHP Settings or something like that and know that they won't be different to a documented standard. Add this labelling to the phpinfo() page and it makes things very very clear what is in play. Richard. -- Richard Quadling Twitter : EE : Zend : PHPDoc @RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY : bit.ly/lFnVea bump -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] set the PHP_INI_ENTRY_* values the same as for php.ini-production
On 03/14/2012 10:09 AM, Ferenc Kovacs wrote: On Mon, Jul 25, 2011 at 12:34 PM, Richard Quadling rquadl...@gmail.comwrote: Maybe, and this is right of the top of my head, if PHP is installed for a production environment with no INI file, or if an ini file doesn't alter any of the core settings (maybe a separation of INI files for core and extensions?), it could be labelled/considered as a virgin PHP install. Something which could be marketed / advertised. Essentially, the PHP Group agree that for a production environment, these are the settings that are the safest to use. If there are considerations that need to be made for shared hosters, then maybe some mechanism to set these appropriately. So, for a user coming to an ISP and looking at hosting, they can see We use Virgin PHP Settings or something like that and know that they won't be different to a documented standard. Add this labelling to the phpinfo() page and it makes things very very clear what is in play. Richard. -- Richard Quadling Twitter : EE : Zend : PHPDoc @RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY : bit.ly/lFnVea bump The biggest problem with the concept of virgin PHP settings being geared for production is that by definition that isn't very developer friendly. Keeping the learning curve shallow has always been a goal for PHP which is why things like display_errors exist. A new developer may not have any idea where to look for PHP errors and might give up when all he gets is a blank page. The assumption is that by the time you are ready to put something into production you have spent a little bit of time with PHP and you likely have stumbled across the suggested production php.ini which is then trivial to apply. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] set the PHP_INI_ENTRY_* values the same as for php.ini-production
On 03/14/2012 10:27 AM, Rasmus Lerdorf wrote: The biggest problem with the concept of virgin PHP settings being geared for production is that by definition that isn't very developer friendly. Keeping the learning curve shallow has always been a goal for PHP which is why things like display_errors exist. A new developer may not have any idea where to look for PHP errors and might give up when all he gets is a blank page. The assumption is that by the time you are ready to put something into production you have spent a little bit of time with PHP and you likely have stumbled across the suggested production php.ini which is then trivial to apply. In that case, perhaps the STD_PHP_INI_* values should match those in php.ini-development? Chris -- Email: christopher.jo...@oracle.com Tel: +1 650 506 8630 Blog: http://blogs.oracle.com/opal/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] set the PHP_INI_ENTRY_* values the same as for php.ini-production
I'm curious: What would be the implications of having a third option to display a generic catch-all error instead of a blank page? For example, something like, An error has occurred. Please check your server's error log for details. That would significantly reduce the confusion factor for inexperienced devs while, at least presumably, not presenting any security risk because no details are actually being included. --Kris On Wed, Mar 14, 2012 at 10:27 AM, Rasmus Lerdorf ras...@lerdorf.com wrote: On 03/14/2012 10:09 AM, Ferenc Kovacs wrote: On Mon, Jul 25, 2011 at 12:34 PM, Richard Quadling rquadl...@gmail.com wrote: Maybe, and this is right of the top of my head, if PHP is installed for a production environment with no INI file, or if an ini file doesn't alter any of the core settings (maybe a separation of INI files for core and extensions?), it could be labelled/considered as a virgin PHP install. Something which could be marketed / advertised. Essentially, the PHP Group agree that for a production environment, these are the settings that are the safest to use. If there are considerations that need to be made for shared hosters, then maybe some mechanism to set these appropriately. So, for a user coming to an ISP and looking at hosting, they can see We use Virgin PHP Settings or something like that and know that they won't be different to a documented standard. Add this labelling to the phpinfo() page and it makes things very very clear what is in play. Richard. -- Richard Quadling Twitter : EE : Zend : PHPDoc @RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY : bit.ly/lFnVea bump The biggest problem with the concept of virgin PHP settings being geared for production is that by definition that isn't very developer friendly. Keeping the learning curve shallow has always been a goal for PHP which is why things like display_errors exist. A new developer may not have any idea where to look for PHP errors and might give up when all he gets is a blank page. The assumption is that by the time you are ready to put something into production you have spent a little bit of time with PHP and you likely have stumbled across the suggested production php.ini which is then trivial to apply. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] set the PHP_INI_ENTRY_* values the same as for php.ini-production
On Wed, Mar 14, 2012 at 6:27 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: On 03/14/2012 10:09 AM, Ferenc Kovacs wrote: On Mon, Jul 25, 2011 at 12:34 PM, Richard Quadling rquadl...@gmail.com wrote: Maybe, and this is right of the top of my head, if PHP is installed for a production environment with no INI file, or if an ini file doesn't alter any of the core settings (maybe a separation of INI files for core and extensions?), it could be labelled/considered as a virgin PHP install. Something which could be marketed / advertised. Essentially, the PHP Group agree that for a production environment, these are the settings that are the safest to use. If there are considerations that need to be made for shared hosters, then maybe some mechanism to set these appropriately. So, for a user coming to an ISP and looking at hosting, they can see We use Virgin PHP Settings or something like that and know that they won't be different to a documented standard. Add this labelling to the phpinfo() page and it makes things very very clear what is in play. Richard. -- Richard Quadling Twitter : EE : Zend : PHPDoc @RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY : bit.ly/lFnVea bump The biggest problem with the concept of virgin PHP settings being geared for production is that by definition that isn't very developer friendly. Keeping the learning curve shallow has always been a goal for PHP which is why things like display_errors exist. A new developer may not have any idea where to look for PHP errors and might give up when all he gets is a blank page. The assumption is that by the time you are ready to put something into production you have spent a little bit of time with PHP and you likely have stumbled across the suggested production php.ini which is then trivial to apply. -Rasmus imo the average developer will have php installed through some wamp stack or the distribution/package manager of his platform, so he/she will have a php.ini with some sane defaults. another idea coming from Philip is to have a php.ini-default which could match with the hard-coded defaults. either way, I think it would be really nice to have a php.ini which we could refer as default in the documentation, and people shouldn't need to look up the php-src to figure out what are the hard-coded defaults and how those are different from the production/development values. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
[PHP-DEV] Let parse_str() parse more than max_input_vars args
It is somewhat unintuitive that parse_str() is subject to the max_input_vars limitation and there are sites that use parse_str() to parse things that aren't directly coming from user query args. There arr two ways to solve this. We could add an optional max_vars arg something along these lines: https://gist.github.com/2038870 The other way to solve this would be to make max_input_vars PHP_INI_ALL and then just let people ini_set() their way around the limit. The one drawback with the optional arg approach is that since parse_str() is a thin layer on top of the query string parser, the error if you exceed the passed max_vars talks about the ini setting which in this case wouldn't really be correct and fixing that would be complicated. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Randomize hash-function in php
Hi, All I just came around that talk a couple of days ago .. http://www.youtube.com/watch?v=R2Cq3CLI6H8 I don't know much about hash-maps and internal php-stuff at all, but they say that the fix provided in 5.3.9 (and 5.4.0) is more a work-around than a fix ... Would it be an option to provide a real fix in PHP 6.0? They got the feedback that this will take some time and is not trivial, but we have a good time before PHP6 and can also break backwards compatibility for php-plugins if really necessary. As they said in the movie, PHP seems to have the algorithm DJBX33A implemented as Ruby. So as they're so proud of the fix provided by the Ruby-Team, may we can use that for PHP as well :) https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2011-4815 This is not much because some attacker can do something, but what if you have a real-world-application that (for some reason) build up an array that just will blow up because of that? I haven't experienced that until now, but it's possible ... Bye Simon
Re: [PHP-DEV] [PATCH] readline extension rl_bind_key addition and other enhancements
Hi,
A few comments on the patch:
A) for licensing reasons we should try to keep as few readline only things as
possible in there (gpl vs. php license)
B) thread safty isn't an issue for readline but you still should do the init
and deinit in rinit/rshutdown not minit/mshutdown, probably also do only
bind_key_functions =NULL in rinit and init the hashtable on demand later.
C) don't compare the r result of zend_hash_find and others against -1 or such
but use SUCCESS/FAILURE
D) i don't really like new features in 5.3 anymore at this stage
E) please log a bug and attach the patch so we can track this
Johannes
P.s. sorry for top posting ...
Osama Abu Elsorour osama.sor...@eformations.net wrote:
I was recently involved in a project that relied heavily on readline to
provide console text input capabilities. However I soon noticed that the
current readline is lacking some important functionality.
This patch applies only to PHP compiled with libreadline (and don't have an
effect if compiled with libeditline). It adds/modified the following:
- Adds support for rl_bind_key function by exposing:
function readline_bind_key_function($key, $callback_func)
where:
$key: Key code to bind to.
$callback_func: A callback function in the form:
function callback($key, $count) where $key and $count are the same parameters
passed from rl_bind_key()
Setting $callback_func to null will cause the binding to be removed for $key.
- Modifies the behavior of readline_info() to allow for setting the readline
properties point and end (it used to only read them but not set them).
Patch below:
--- php-5.3.10/ext/readline/readline.c 2012-01-01 15:15:04.0 +0200
+++ php-5.3.10-patched/ext/readline/readline.c 2012-03-13 09:35:43.0
+0200
@@ -57,9 +57,12 @@ PHP_FUNCTION(readline_callback_read_char
PHP_FUNCTION(readline_callback_handler_remove);
PHP_FUNCTION(readline_redisplay);
PHP_FUNCTION(readline_on_new_line);
+PHP_FUNCTION(readline_bind_key_function);
static zval *_prepped_callback = NULL;
+static HashTable *_bind_key_functions_ht = NULL;
+
#endif
static zval *_readline_completion = NULL;
@@ -121,6 +124,12 @@ ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_INFO(arginfo_readline_on_new_line, 0)
ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_readline_bind_key_function, 0, 0, 2)
+ ZEND_ARG_INFO(0, key)
+ ZEND_ARG_INFO(0, funcname)
+ZEND_END_ARG_INFO()
+
#endif
/* }}} */
@@ -142,6 +151,7 @@ static const zend_function_entry php_rea
PHP_FE(readline_callback_handler_remove,
arginfo_readline_callback_handler_remove)
PHP_FE(readline_redisplay, arginfo_readline_redisplay)
PHP_FE(readline_on_new_line, arginfo_readline_on_new_line)
+ PHP_FE(readline_bind_key_function, arginfo_readline_bind_key_function)
#endif
PHP_FE_END
};
@@ -165,6 +175,9 @@ ZEND_GET_MODULE(readline)
PHP_MINIT_FUNCTION(readline)
{
+ ALLOC_HASHTABLE(_bind_key_functions_ht);
+ zend_hash_init(_bind_key_functions_ht, 255, NULL, ZVAL_PTR_DTOR, 0);
+
using_history();
return SUCCESS;
}
@@ -181,6 +194,9 @@ PHP_RSHUTDOWN_FUNCTION(readline)
zval_ptr_dtor(_prepped_callback);
_prepped_callback = 0;
}
+
+ zend_hash_destroy(_bind_key_functions_ht);
+ FREE_HASHTABLE(_bind_key_functions_ht);
#endif
return SUCCESS;
@@ -254,8 +270,16 @@ PHP_FUNCTION(readline_info)
}
RETVAL_STRING(SAFE_STRING(oldstr),1);
} else if (!strcasecmp(what, point)) {
+ if (value) {
+ convert_to_long_ex(value);
+ rl_point = Z_LVAL_PP(value);
+ }
RETVAL_LONG(rl_point);
} else if (!strcasecmp(what, end)) {
+ if (value) {
+ convert_to_long_ex(value);
+ rl_end = Z_LVAL_PP(value);
+ }
RETVAL_LONG(rl_end);
#ifdef HAVE_LIBREADLINE
} else if (!strcasecmp(what, mark)) {
@@ -612,6 +636,74 @@ PHP_FUNCTION(readline_on_new_line)
}
/* }}} */
+/* {{{ proto bool readline_bind_key_function(string funcname)
+ Readline rl_bind_key */
+
+static int _readline_bind_key_cb(int count, int key)
+{
+ zval *params[2];
+ TSRMLS_FETCH();
+
+ params[0]=_readline_long_zval(key);
+ params[1]=_readline_long_zval(count);
+
+ zval **_callback_func = NULL;
+ long _key = key;
+ int r = zend_hash_find(_bind_key_functions_ht, (char *) _key,
sizeof(_key), (void **)_callback_func);
+ if (r == -1)
+ return -1;
+
+ int _return_int = 0;
+ zval *_callback_return = NULL;
+ MAKE_STD_ZVAL(_callback_return);
+ if (call_user_function(CG(function_table), NULL, *_callback_func,
_callback_return, 2, params TSRMLS_CC) == SUCCESS) {
+
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
hi Rasmus, As the ini_all option sounds appealing, I can imagine ISPs willing to do not allow their users to change this value, and that's something I would not allow random users either. I'd to go with the optional argument, adding a clear in the documentation about the confusing error message. Cheers, On Wed, Mar 14, 2012 at 8:42 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: It is somewhat unintuitive that parse_str() is subject to the max_input_vars limitation and there are sites that use parse_str() to parse things that aren't directly coming from user query args. There arr two ways to solve this. We could add an optional max_vars arg something along these lines: https://gist.github.com/2038870 The other way to solve this would be to make max_input_vars PHP_INI_ALL and then just let people ini_set() their way around the limit. The one drawback with the optional arg approach is that since parse_str() is a thin layer on top of the query string parser, the error if you exceed the passed max_vars talks about the ini setting which in this case wouldn't really be correct and fixing that would be complicated. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] readline extension rl_bind_key addition and other enhancements
On Tue, Mar 13, 2012 at 10:46 AM, Osama Abu Elsorour osama.sor...@eformations.net wrote: [snip] + zval *_temp_callback = NULL; + MAKE_STD_ZVAL(_temp_callback); + *_temp_callback = *arg; + zval_copy_ctor(_temp_callback); May I ask where you got this pattern for copying zvals from? Is this kind of code shown in some tutorial / manual / etc? Such code is used all other the place, instead of the MAKE_COPY_ZVAL macro and I wonder why. Doing manual copying has a good chance of leaking memory. Just from glancing I'd say that your code will leak if arg has a refcount 1 (but I could be wrong) as it does not PINIT the zval after it was copied. Nikita -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
On 03/14/2012 01:32 PM, Pierre Joye wrote: hi Rasmus, As the ini_all option sounds appealing, I can imagine ISPs willing to do not allow their users to change this value, and that's something I would not allow random users either. I'd to go with the optional argument, adding a clear in the documentation about the confusing error message. But Pierre, you understand that by the time you ini_set() it in the code it can only ever affect parse_str() calls. Normal GPC parsing is done prior to the PHP script running so there is no way for a userspace script to ini_set() themselves to a state where they will be insecure to a remote attack. They would have to go out of their way to specifically write code to do that and that is something they can obviously do anyway by simply building a big hash from some external source. So I don't really think this is a valid concern. If this was a real concern I would think you would have objected to the current INI_PERDIR. This is where a user can make his scripts unsafe by disabling max_input_vars in a .htaccess file. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
But Pierre, you understand that by the time you ini_set() it in the code it can only ever affect parse_str() calls. Well, wouldn't INI_ALL would allow .htaccess files to override that statement, and hence open the vulnerability? Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
hi, On Wed, Mar 14, 2012 at 10:38 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: On 03/14/2012 01:32 PM, Pierre Joye wrote: hi Rasmus, As the ini_all option sounds appealing, I can imagine ISPs willing to do not allow their users to change this value, and that's something I would not allow random users either. I'd to go with the optional argument, adding a clear in the documentation about the confusing error message. I would think you would have objected to the current INI_PERDIR. This is where a user can make his scripts unsafe by disabling max_input_vars in a .htaccess file. I was sure it was system and not per dir. The ini all option can't hurt more then. Cheers, -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
On 03/14/2012 02:46 PM, Anthony Ferrara wrote: But Pierre, you understand that by the time you ini_set() it in the code it can only ever affect parse_str() calls. Well, wouldn't INI_ALL would allow .htaccess files to override that statement, and hence open the vulnerability? No because it is already PERDIR. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
Hi! The other way to solve this would be to make max_input_vars PHP_INI_ALL and then just let people ini_set() their way around the limit. I think making it PHP_INI_ALL is OK. If you have access to a way to change INI_ALL vars, that means you can run code on that system, then DoS protection is meaningless on this stage. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
On 14/03/12 20:42, Rasmus Lerdorf wrote: It is somewhat unintuitive that parse_str() is subject to the max_input_vars limitation and there are sites that use parse_str() to parse things that aren't directly coming from user query args. There arr two ways to solve this. We could add an optional max_vars arg something along these lines: https://gist.github.com/2038870 The other way to solve this would be to make max_input_vars PHP_INI_ALL and then just let people ini_set() their way around the limit. The one drawback with the optional arg approach is that since parse_str() is a thin layer on top of the query string parser, the error if you exceed the passed max_vars talks about the ini setting which in this case wouldn't really be correct and fixing that would be complicated. -Rasmus Configuring it through ini_set would be a hack. +1 for doing it by a new str_parse() parameter. I'm not really keen of implementing that setting and restoring PG(max_input_vars), but as a fast fix, it's ok. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
On 03/14/2012 03:11 PM, Stas Malyshev wrote: Hi! The other way to solve this would be to make max_input_vars PHP_INI_ALL and then just let people ini_set() their way around the limit. I think making it PHP_INI_ALL is OK. If you have access to a way to change INI_ALL vars, that means you can run code on that system, then DoS protection is meaningless on this stage. I ran into this in some existing code that broke upgrading to 5.3.10. It was a backend call to an API where the API result was being fed to parse_str(). The API itself is trusted, so no risk of a HashDoS from it. Other than replacing the call to parse_str() with a similar userspace implementation there was no way to fix this safely. I could do a .htaccess for just that URI, but that would open up the frontend of this particular request to a HashDoS. I'll make the INI_ALL change for the next release. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
Sounds like a least dangerous way of solving the problem to me. The only issue I can see with this fix is what would happen is if after the PG(max_input_vars) = max_vars; call the request got interrupted in persistent environment such as Apache (mod_php). Wouldn't that means that for subsequent requests the value would not be equal to the one set by the user? On Wed, Mar 14, 2012 at 3:42 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: It is somewhat unintuitive that parse_str() is subject to the max_input_vars limitation and there are sites that use parse_str() to parse things that aren't directly coming from user query args. There arr two ways to solve this. We could add an optional max_vars arg something along these lines: https://gist.github.com/2038870 The other way to solve this would be to make max_input_vars PHP_INI_ALL and then just let people ini_set() their way around the limit. The one drawback with the optional arg approach is that since parse_str() is a thin layer on top of the query string parser, the error if you exceed the passed max_vars talks about the ini setting which in this case wouldn't really be correct and fixing that would be complicated. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] default charset confusion
On 13/03/12 00:25, Stas Malyshev wrote: Hi! Still, that API is likely wrong: a library function written by someone completely unrelated to the main application shouldn't be echoing anything through the output. And if it's not generating the html, the htmlspecialchars is better done from the return at the calling application (probably after converting the internal charset). Again, you making a huge amount of assumptions about how ALL the applications must work, which means you are wrong in 99.(9)% of cases, because there's infinitely many applications which don't work exactly like yours does, and we have no idea how they work. No. I'm saying how I consider they should work, saying that an API doing otherwise is likely* wrong (aka. has a bad design), very much as I'd consider insane a company policy stating PHP function arguments shall be named $a, $b, $c That's obviously my opinion, but I think most applications will conform to that, just as most apps will use more descriptive argument names than $c**. * There might be some very very special application where it turns out to be an appropiate design, but that would be the exception. ** Even though there are 26!/(26-n)! ways to name so badly the arguments of a n-ary function. The main point is that having global state (and yet worse, changeable global state) significantly influence how basic functions are working is dangerous. It's like keeping everything in globals and instead of passing parameters between functions just change some globals and expect functions to pick it up. I agree with you, in the general case. Yet, I consider the html charset to be a global state. And passing the global variables as parameters on each function call would be nearly as bad as passing parameters as globals. I just positioned the opposite way for parse_str(), while being fully aware of that. Such interfaces may be well served by switching the setting many times. That's exactly what I am trying to avoid, and you are just illustrating why this proposal is dangerous - because that's exactly what is going to happen in the code, instead of passing proper arguments to htmlspecialchars people will start changing INI settings left and right, and then nobody would know what htmlspecialchars() call actually does without tracking all the INI changes along the way. That's assuming people would need to use different output charsets, which I don't consider to be the case. How many people is using now the third htmlspecialchars() parameter? What makes you think that they would need to change the default global, *several times per request*? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
On 03/14/2012 04:27 PM, Ilia Alshanetsky wrote: Sounds like a least dangerous way of solving the problem to me. The only issue I can see with this fix is what would happen is if after the PG(max_input_vars) = max_vars; call the request got interrupted in persistent environment such as Apache (mod_php). Wouldn't that means that for subsequent requests the value would not be equal to the one set by the user? Yes, it would need a zend_alter_ini_entry_ex() call there. The patch wasn't complete, just a quick hack. But it would still have an out-of-context error message when you exceed it. At least with a userspace ini_set() the error would make sense. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
On Thu, Mar 15, 2012 at 7:38 AM, Rasmus Lerdorf ras...@lerdorf.com wrote: Yes, it would need a zend_alter_ini_entry_ex() call there. The patch wasn't complete, just a quick hack. But it would still have an out-of-context error message when you exceed it. At least with a userspace ini_set() the error would make sense. As mentioned on IRC, a function signature of array parse_urlencoded(string $s) would be useful too; the separated logic would allow for avoiding max_input_vars altogether and not having to worry about parameter name mangling (variable name rules). The nasty part is that much of the treat_data code would have to be duplicated (I think). Besides that, applying the hash randomisation patch to only userland arrays would make the max_input_vars less critical and at the same time avoid breaking opcode caches and other low-level dependencies. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
On 03/14/2012 07:34 PM, Tjerk Anne Meesters wrote: On Thu, Mar 15, 2012 at 7:38 AM, Rasmus Lerdorf ras...@lerdorf.com wrote: Yes, it would need a zend_alter_ini_entry_ex() call there. The patch wasn't complete, just a quick hack. But it would still have an out-of-context error message when you exceed it. At least with a userspace ini_set() the error would make sense. As mentioned on IRC, a function signature of array parse_urlencoded(string $s) would be useful too; the separated logic would allow for avoiding max_input_vars altogether and not having to worry about parameter name mangling (variable name rules). The nasty part is that much of the treat_data code would have to be duplicated (I think). Besides that, applying the hash randomisation patch to only userland arrays would make the max_input_vars less critical and at the same time avoid breaking opcode caches and other low-level dependencies. Sure, but this is a longer-term fix. Right now I am more concerned about the fact that we broke code that worked fine in PHP 5.3.8 without any way to make it work safely. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Let parse_str() parse more than max_input_vars args
As mentioned on IRC, a function signature of array
parse_urlencoded(string $s) would be useful too; the separated logic
would allow for avoiding max_input_vars altogether and not having to
worry about parameter name mangling (variable name rules). The nasty
part is that much of the treat_data code would have to be duplicated
(I think).
Besides that, applying the hash randomisation patch to only userland
arrays would make the max_input_vars less critical and at the same
time avoid breaking opcode caches and other low-level dependencies.
Sure, but this is a longer-term fix. Right now I am more concerned about
the fact that we broke code that worked fine in PHP 5.3.8 without any
way to make it work safely.
I guess this looks acceptable then:
ini_set('max_input_vars', 5000);
parse_str($s, $arr);
ini_restore('max_input_vars');
Although, arguably the last statement would not be needed, since all
input has already been processed ;-)
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
