Restored FFI to working order (#28)

* Restored FFI to working order

* Disabled incompatible tests, fixed string conversion

* Use extension_loaded instead of checking PHP version

* Added better checks for FFI

* Do not build FFI deps if no FFI is present

.gitignore

@@ -199,3 +199,6 @@ Makefile*
 .DS_Store
 composer.lock
 PHP-Parser*/
+src/*.so
+src/*.dylib
+src/*.dll

Makefile.frag

@@ -4,6 +4,7 @@ DEPS_DIR=$(BUILD_DIR)/deps
 INT_DIR=$(BUILD_DIR)/install
 INSTALL_DIR=$(shell pwd)
 GENERATE_STUBS=$(shell expr `php --version | head -1 | cut -f 2 -d' '` \>= 7.1)
+HAS_FFI=$(shell php -m | grep FFI | wc -l | xargs)
 
 CMAKE = cmake3
 ifeq (, $(shell which cmake3))
@@ -15,13 +16,8 @@ CMAKE_BUILD = $(CMAKE) --build
 CMAKE_BUILD_TYPE ?= RelWithDebInfo
 CMAKE_TARGET = --config $(CMAKE_BUILD_TYPE) --target install
 
-# configure for shared aws-crt-ffi.so
+all: extension ffi
-$(BUILD_DIR)/aws-crt-ffi-shared/CMakeCache.txt:
+.PHONY: all extension ffi
-	$(CMAKE_CONFIGURE) -Hcrt/aws-crt-ffi -Bbuild/aws-crt-ffi-shared -DBUILD_SHARED_LIBS=ON
-
-# build shared libaws-crt-ffi.so
-$(BUILD_DIR)/aws-crt-ffi-shared/libaws-crt-ffi.so: $(BUILD_DIR)/aws-crt-ffi-shared/CMakeCache.txt
-	$(CMAKE_BUILD) build/aws-crt-ffi-shared $(CMAKE_TARGET)
 
 # configure for static aws-crt-ffi.a
 $(BUILD_DIR)/aws-crt-ffi-static/CMakeCache.txt:
@@ -32,7 +28,7 @@ $(BUILD_DIR)/aws-crt-ffi-static/libaws-crt-ffi.a: $(BUILD_DIR)/aws-crt-ffi-stati
 	$(CMAKE_BUILD) build/aws-crt-ffi-static $(CMAKE_TARGET)
 
 # PHP extension target
-awscrt: ext/awscrt.lo
+extension: ext/awscrt.lo
 
 # Force the crt object target to depend on the CRT static library
 ext/awscrt.lo: ext/awscrt.c
@@ -56,13 +52,37 @@ ext/api.h : src/api.h
 ext/php_aws_crt.h: ext/awscrt_arginfo.h ext/api.h
 
 # FFI target
-ffi: src/libaws-crt-ffi.so
+ffi: src/api.h src/libaws-crt-ffi.$(SHLIB_SUFFIX_NAME)
+
+# configure for shared aws-crt-ffi.so
+$(BUILD_DIR)/aws-crt-ffi-shared/CMakeCache.txt:
+ifeq ($(HAS_FFI),1)
+	$(CMAKE_CONFIGURE) -Hcrt/aws-crt-ffi -Bbuild/aws-crt-ffi-shared -DBUILD_SHARED_LIBS=ON
+endif
+
+# build shared libaws-crt-ffi.so
+$(BUILD_DIR)/aws-crt-ffi-shared/libaws-crt-ffi.$(SHLIB_SUFFIX_NAME): $(BUILD_DIR)/aws-crt-ffi-shared/CMakeCache.txt
+ifeq ($(HAS_FFI),1)
+	$(CMAKE_BUILD) build/aws-crt-ffi-shared $(CMAKE_TARGET)
+endif
 
 # copy the lib into the src folder
-modules/libaws-crt-ffi.so: $(BUILD_DIR)/aws-crt-ffi-shared/libaws-crt-ffi.so src/api.h
+src/libaws-crt-ffi.$(SHLIB_SUFFIX_NAME): $(BUILD_DIR)/aws-crt-ffi-shared/libaws-crt-ffi.$(SHLIB_SUFFIX_NAME) src/api.h
-	cp -v $(BUILD_DIR)/aws-crt-ffi-shared/libaws-crt-ffi.so modules/libaws-crt-ffi.so
+ifeq ($(HAS_FFI),1)
+	cp -v $(BUILD_DIR)/aws-crt-ffi-shared/libaws-crt-ffi.$(SHLIB_SUFFIX_NAME) src/libaws-crt-ffi.$(SHLIB_SUFFIX_NAME)
+endif
+
+vendor/bin/phpunit:
+	composer update
+
+test-ffi: vendor/bin/phpunit ffi
+ifeq ($(HAS_FFI),1)
+	AWS_CRT_PHP_FFI=1 composer run test-ffi
+endif
+
+test-extension: vendor/bin/phpunit extension
+	AWS_CRT_PHP_EXTENSION=1 composer run test-extension
 
 # Use PHPUnit to run tests
-test: ext/api.h ext/awscrt_arginfo.h ext/awscrt.lo
+test: test-ffi test-extension
-	composer update
+
-	composer run test

builder.json

@@ -32,6 +32,6 @@
         "NO_INTERACTION": "1"
     },
     "test_steps": [
-        ["make", "test"]
+        ["./ci-test.sh"]
     ]
 }

ci-test.sh

@@ -0,0 +1,11 @@
+#!/usr/bin/env bash
+
+set -ex
+
+HAS_FFI=$(php -m | grep FFI | wc -l | xargs)
+
+make test-extension
+
+if [[ $HAS_FFI -gt 0 ]]; then
+  make test-ffi
+fi

composer.json

@@ -29,7 +29,9 @@
         ]
     },
     "scripts": {
-        "test": "@php -d extension=modules/awscrt.so vendor/bin/phpunit tests"
+        "test": "@php -d extension=modules/awscrt.so vendor/bin/phpunit tests",
+        "test-extension": "@php -d extension=modules/awscrt.so vendor/bin/phpunit tests",
+        "test-ffi": "@php vendor/bin/phpunit tests"
     },
     "license": "Apache-2.0"
 }

ext/signing.c

@@ -258,8 +258,9 @@ PHP_FUNCTION(aws_crt_sign_request_aws) {
     zend_ulong php_signable = 0;
     zend_ulong php_signing_config = 0;
     zval *php_on_complete = 0;
+    zend_ulong php_user_data = 0;
 
-    aws_php_parse_parameters("llz", &php_signable, &php_signing_config, &php_on_complete);
+    aws_php_parse_parameters("llzl", &php_signable, &php_signing_config, &php_on_complete, &php_user_data);
 
     aws_crt_signable *signable = (void *)php_signable;
     aws_crt_signing_config_aws *signing_config = (void *)php_signing_config;

src/AWS/CRT/Auth/Signing.php

@@ -13,6 +13,6 @@ abstract class Signing extends NativeResource {
             function($result, $error_code) use ($on_complete) {
                 $signing_result = SigningResult::fromNative($result);
                 $on_complete($signing_result, $error_code);
-            });
+            }, null);
     }
 }

src/AWS/CRT/CRT.php

@@ -24,6 +24,14 @@ final class CRT {
         if (is_null(self::$impl)) {
             // Figure out what backends are/should be available
             $backends = ['Extension'];
+            if (extension_loaded('ffi')) {
+                $backends = ['Extension', 'FFI'];
+                if (getenv('AWS_CRT_PHP_EXTENSION')) {
+                    $backends = ['Extension'];
+                } else if (getenv('AWS_CRT_PHP_FFI')) {
+                    $backends = ['FFI'];
+                }
+            }
 
             // Try to load each backend, give up if none succeed
             $exceptions = [];
@@ -31,6 +39,7 @@ final class CRT {
                 try {
                     $backend = 'AWS\\CRT\\Internal\\' . $backend;
                     self::$impl = new $backend();
+                    break;
                 } catch (RuntimeException $rex) {
                     array_push($exceptions, $rex);
                 }
@@ -67,6 +76,20 @@ final class CRT {
         }
     }
 
+    /**
+     * @return bool true if using PHP FFI (PHP 7.4+)
+     */
+    public static function isFFI() {
+        return self::isLoaded() && strstr(get_class(self::$impl), 'FFI');
+    }
+
+    /**
+     * @return bool true if using PHP Extension awscrt
+     */
+    public static function isExtension() {
+        return self::isLoaded() && strstr(get_class(self::$impl), 'Extension');
+    }
+
     /**
      * @return integer last error code reported within the CRT
      */
@@ -319,7 +342,7 @@ final class CRT {
             $signing_result, $http_message);
     }
 
-    function sign_request_aws($signable, $signing_config, $on_complete) {
+    function sign_request_aws($signable, $signing_config, $on_complete, $user_data) {
-        return self::$impl->aws_crt_sign_request_aws($signable, $signing_config, $on_complete);
+        return self::$impl->aws_crt_sign_request_aws($signable, $signing_config, $on_complete, $user_data);
     }
 }

src/AWS/CRT/Internal/FFI.php

@@ -0,0 +1,82 @@
+<?php
+
+namespace AWS\CRT\Internal;
+
+use Exception;
+use RuntimeException;
+
+/**
+ * @internal
+ * Forwards calls on to libaws-crt-ffi via FFI
+ */
+final class FFI {
+    private static $ffi = null;
+    private static $refcount = 0;
+
+    function __construct() {
+        if (is_null(self::$ffi)) {
+            try {
+                $shlib_ext = 'so';
+                $uname_s = php_uname('s');
+                if ($uname_s == 'Darwin') {
+                    $shlib_ext = 'dylib';
+                } else if ($uname_s == 'WINNT') {
+                    $shlib_ext = 'dll';
+                }
+                self::$ffi = \FFI::cdef(
+                    file_get_contents(__DIR__ . "/../../../api.h"),
+                    __DIR__ . "/../../../libaws-crt-ffi." . $shlib_ext);
+                self::init();
+            } catch (Exception $e) {
+                throw new RuntimeException('Exception while initializing CRT via FFI', 0, $e);
+            }
+        }
+        ++self::$refcount;
+    }
+
+    function __destruct() {
+        if (--self::$refcount == 0) {
+            self::clean_up();
+            self::$ffi = null;
+        }
+    }
+
+    /**
+     * Forwards any call made on this object to the FFI function of the
+     * same name with the supplied arguments. Argument type hinting and checking
+     * occurs at the CRT wrapper.
+     */
+    function __call(string $name, $args) {
+        // Expand strings to (string, length)
+        $ffi_args =[];
+        foreach ($args as $arg) {
+            if (is_string($arg)) {
+                $len = strlen($arg);
+                if ($len > 0) {
+                    $uint8_t = \FFI::type('uint8_t');
+                    $uint8_array = \FFI::arrayType($uint8_t, [$len]);
+                    $buf = \FFI::new($uint8_array);
+                    \FFI::memcpy($buf, $arg, $len);
+                    $ffi_args [] = $buf;
+                    $ffi_args [] = $len;
+                } else {
+                    $ffi_args [] = null;
+                    $ffi_args [] = 0;
+                }
+            } else if (is_resource($arg)) {
+                throw new RuntimeException("Resource types are not supported for FFI");
+            } else {
+                $ffi_args []= $arg;
+            }
+        }
+        return call_user_func_array([self::$ffi, $name], $ffi_args);
+    }
+
+    private static function init() {
+        return self::$ffi->aws_crt_init();
+    }
+
+    private static function clean_up() {
+        return self::$ffi->aws_crt_clean_up();
+    }
+}

tests/SigningTest.php

@@ -39,6 +39,7 @@ final class SigningTest extends CrtTestCase {
     }
 
     public function testSignableFromChunkLifetime() {
+        $this->skipFFI();
         $chunk = "THIS IS A TEST CHUNK IT CONTAINS MULTITUDES";
         $stream = fopen("php://memory", 'r+');
         fputs($stream, $chunk);
@@ -61,6 +62,8 @@ final class SigningTest extends CrtTestCase {
     const SIGV4TEST_SERVICE = 'service';
     const SIGV4TEST_REGION = 'us-east-1';
     public function testSigv4HeaderSigning() {
+        $this->skipFFI();
+
         $date = mktime(12, 36, 0, 8, 30, 2015);
         $credentials_provider = new StaticCredentialsProvider([
             'access_key_id' => self::SIGV4TEST_ACCESS_KEY_ID,

tests/StreamTest.php

@@ -19,6 +19,7 @@ final class InputStreamTest extends CrtTestCase {
     }
 
     public function testMemoryStream() {
+        $this->skipFFI();
         $mem_stream = $this->getMemoryStream();
         $stream = new InputStream($mem_stream);
         $this->assertNotNull($stream, "Failed to create InputStream from PHP memory stream");

tests/common.inc

@@ -29,4 +29,10 @@ abstract class CrtTestCase extends PHPUnit_Framework_TestCase {
             $this->setExpectedException($arguments[0]);
         }
     }
+
+    public function skipFFI() {
+        if (CRT::isFFI()) {
+            $this->markTestSkipped();
+        }
+    }
 }