#170 replace eval(): Improving HHVM RA support and possibly security

[Colin Kiegel]

Current Behaviour:
* phplot currently uses eval() in SetDashedStyle(). This has some drawbacks - evals can easily turn into serious security holes, even if you are super careful & eval is not supported with HipHopVirtualMachine RepoAuthoritative Mode.

Ok HHVM RA might not be what everyone is using today, but it is fast and I would like to use it on a big project relying on phplot. This is why I created this patch and provide it upstream. It would be nice, if you could include it. It's a really small change - but one that counts. :-)

Thanks.

[lbayuk]

I never liked that eval(), although I do not believe there is any possible security issue. It is a little too 'tricky' for me, and took me too long to figure out how it worked back when I was documenting things... So yes, I would like to get rid of it (although I've never heard of HHVM RA).

Thanks for the patch, but first I want to look for something a little simpler. If I can come up with something that doesn't use eval() and is more straight-forward, as well as working with older PHP versions, I think that would be better.

(By the way, I am pretty sure your 2 json_encode() calls can be removed. Whether in truecolor or indexed color modes, the GD color values are just integers.)

[Colin Kiegel]

Thanks for the quick reply. The approach of the patch was "to be minimally intrusive". And yes, the json_encode is obsolete in case of integers.

1. If you like object oriented style, you could make the style an object, with setter-methods for columns and colour.

2. Or if you still want to keep the change small, just create an array and iterate over the array and replace values inside the array.

3. .. or first store only the column-information in $this and delay the calculation of the default_style until the color is known.

These are the alternatives that come to my mind. :-)

PS: HHVM RA compiles PHP to 'opcode-repositories' which do not allow any dynamic code generation after compilation. Wikipedia for example recently switched from PHP.net/Zend-Engine to HHVM (but without RA mode): http://hhvm.com/blog/7205/wikipedia-on-hhvm

[lbayuk]

We are thinking alike. Your '2' is similar to what I am trying. Use an array instead of a code string for the 'template', with a marker for the color to be substituted. The constant for transparent pixels can be inserted at this stage. For example, for the pattern '2-1' it would be array('#','#',-6), with # for the color to be inserted later, and -6 happens to be IMG_COLOR_TRANSPARENT.

Then replace the marker with the applicable color when the array is needed for imagesetstyle(). No need to iterate over it, as str_replace works on arrays too.

For setting up the initial template array, I'm using array_merge() to build it. I don't like it, but I can't find a clean way to concatenate or append numeric indexed arrays in PHP.

[Colin Kiegel]

cool, sounds good. Lightyears better than eval. :-)

[lbayuk]

Oops... the risk of distraction. The eval() removal fix isn't hard, but I started looking at dashed line setup, and wondered why it doesn't allow more than one dashed pattern to be used in a plot. That actually looks fixable, and is related to removing the eval().

But then I dug up old notes on how the dashed patterns don't really display right - the patterns don't seem to be continuing at the data points. I wrote that up as a new bug report for some day - it's harder.

OK, I'm back on removing eval(). Turns out there is a better way than using an array for the 'template': just a string, with space-separated values, use str_replace() then explode() to make the pattern array for GD. It's faster, more memory efficient, fewer PHP lines, and easier to understand. I'm going to put it into PHPlot and test it out.

[lbayuk]

Removed eval() by using a string template instead of a code string for the dashed line pattern.

Changes were committed to SVN and will be in the next release.

[Colin Kiegel]

Thank you, thats great. Is there any approximate date or schedule for the next release? I would like to check back here, as soon as there is a chance for a new release. A rough hint would already be perfect. :-)

[lbayuk]

Fixed in 6.2.0