From bcf0e9ac16c9b6160b4fd11ea858518ccd3506c2 Mon Sep 17 00:00:00 2001 From: Alex Palaistras Date: Tue, 26 Jan 2016 16:26:42 +0000 Subject: [PATCH 1/3] Context: Fix variable scoping issues for `Eval` Previously, `Context.Eval` relied on Zend's `zend_eval_script` function for executing arbitrary strings. Unfortunately, the above contains a curious bit of functionality where the string passed is prepended with a `return` statement depending on whether or not a zval is passed for writing the return value. This, of course breaks all but the simplest scripts, as we always write the return value to a zval (whether the user calling `Context.Eval` is to use the return value is another matter entirely). This commit changes the implementation to a more direct (and therefore more controllable) approach. Some tests have been changed to cover this bug. --- engine/context.c | 26 ++++++++++++++---------- engine/context.go | 6 +----- engine/context_test.go | 12 +++-------- engine/include/php5/_context.h | 35 +++++++++++++++++++++++++++++++++ engine/include/php7/_context.h | 15 ++++++++++++++ engine/include/php7/_receiver.h | 13 ++++++------ 6 files changed, 77 insertions(+), 30 deletions(-) diff --git a/engine/context.c b/engine/context.c index 115bb80..c5b73bf 100644 --- a/engine/context.c +++ b/engine/context.c @@ -65,22 +65,28 @@ void context_exec(engine_context *context, char *filename) { } void *context_eval(engine_context *context, char *script) { - int status; - zval tmp; + zval str; + VALUE_SET_STRING(&str, script); - // Attempt to evaluate inline script. - zend_first_try { - status = zend_eval_string(script, &tmp, "gophp-engine"); - } zend_catch { - errno = 1; - return NULL; - } zend_end_try(); + // Compile script value. + uint32_t compiler_options = CG(compiler_options); + CG(compiler_options) = ZEND_COMPILE_DEFAULT_FOR_EVAL; + zend_op_array *op = zend_compile_string(&str, "gophp-engine"); + CG(compiler_options) = compiler_options; + + zval_dtor(&str); - if (status == FAILURE) { + // Return error if script failed to compile. + if (!op) { errno = 1; return NULL; } + // Attempt to execute compiled string. + zval tmp; + CONTEXT_EXECUTE(op, &tmp); + + // Allocate result value and copy temporary execution result in. zval *result = malloc(sizeof(zval)); value_copy(result, &tmp); diff --git a/engine/context.go b/engine/context.go index f3ad9d8..1a7afe3 100644 --- a/engine/context.go +++ b/engine/context.go @@ -90,11 +90,7 @@ func (c *Context) Exec(filename string) error { // containing the PHP value returned by the expression, if any. Any output // produced is written context's pre-defined io.Writer instance. func (c *Context) Eval(script string) (*Value, error) { - // When PHP compiles code with a non-NULL return value expected, it simply - // prepends a `return` call to the code, thus breaking simple scripts that - // would otherwise work. Thus, we need to wrap the code in a closure, and - // call it immediately. - s := C.CString("call_user_func(function(){" + script + "});") + s := C.CString(script) defer C.free(unsafe.Pointer(s)) result, err := C.context_eval(c.context, s) diff --git a/engine/context_test.go b/engine/context_test.go index e81562c..e3faae1 100644 --- a/engine/context_test.go +++ b/engine/context_test.go @@ -260,19 +260,14 @@ func TestContextBind(t *testing.T) { c, _ := NewContext() c.Output = &w - script, err := NewScript("bind.php", "obj, t); \ - object_properties_init(&r->obj, t); \ - r->obj.handlers = &receiver_handlers; \ - return &r->obj; \ +#define RECEIVER_OBJECT_CREATE(r, t) do { \ + r = emalloc(sizeof(engine_receiver)); \ + memset(r, 0, sizeof(engine_receiver)); \ + zend_object_std_init(&r->obj, t); \ + object_properties_init(&r->obj, t); \ + r->obj.handlers = &receiver_handlers; \ + return &r->obj; \ } while (0) #define RECEIVER_OBJECT_DESTROY(r) do { \ From 976e5fe13897555583d596782d5cdd6ee32c51b8 Mon Sep 17 00:00:00 2001 From: Alex Palaistras Date: Tue, 26 Jan 2016 16:48:08 +0000 Subject: [PATCH 2/3] README: Add additional example for binding and returning values --- README.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/README.md b/README.md index 92254b4..4ca8600 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,37 @@ func main() { The above will execute script file `index.php` located in the current folder and will write any output to the `io.Writer` assigned to `Context.Output` (in this case, the standard output). +### Binding and returning variables + +The following example demonstrates binding a Go variable to the running PHP context, and returning a PHP variable for use in Go: + +```go +package main + +import ( + "fmt" + php "github.com/deuill/go-php" +) + +func main() { + engine, _ := php.New() + context, _ := engine.NewContext() + + var str string = "Hello" + context.Bind("var", str) + + val, _ := context.Eval("return $var.' World';") + fmt.Printf("%s", val.Interface()) + // Prints 'Hello World' back to the user. + + engine.Destroy() +} +``` + +A string value "Hello" is attached using `Context.Bind` under a name `var` (available in PHP as `$var`). A script is executed inline using `Context.Eval`, combinding the attached value with a PHP string and returning it to the user. + +Finally, the value is returned as an `interface{}` using `Value.Interface()` (one could also use `Value.String()`, though the both are equivalent in this case). + ## License All code in this repository is covered by the terms of the MIT License, the full text of which can be found in the LICENSE file. From f578e8241b1edd52983cab126522b47624f2a3a8 Mon Sep 17 00:00:00 2001 From: Alex Palaistras Date: Tue, 26 Jan 2016 16:48:37 +0000 Subject: [PATCH 3/3] Context: Fix memory leak when not using `Eval` return value The `Context.Eval` function invariably creates a `Value` which may or may not end up being passed to the caller. Keep a reference to the value and destroy it during `Context.Destroy` at the latest. --- engine/context.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/engine/context.go b/engine/context.go index 1a7afe3..577716a 100644 --- a/engine/context.go +++ b/engine/context.go @@ -31,7 +31,7 @@ type Context struct { Header http.Header context *C.struct__engine_context - values map[string]*Value + values []*Value } // NewContext creates a new execution context for the active engine and returns @@ -39,7 +39,7 @@ type Context struct { func NewContext() (*Context, error) { ctx := &Context{ Header: make(http.Header), - values: make(map[string]*Value), + values: make([]*Value, 0), } ptr, err := C.context_new(unsafe.Pointer(ctx)) @@ -66,7 +66,7 @@ func (c *Context) Bind(name string, val interface{}) error { defer C.free(unsafe.Pointer(n)) C.context_bind(c.context, n, v.Ptr()) - c.values[name] = v + c.values = append(c.values, v) return nil } @@ -105,6 +105,8 @@ func (c *Context) Eval(script string) (*Value, error) { return nil, err } + c.values = append(c.values, val) + return val, nil }