Implemented async callbacks to PHP regardless of thread model, Sigv4 signing #24

justinboswell · 2021-05-19T00:36:55Z

Depends on:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…http message signing

…#25) * Broke C code into separate files, moved common stuff to php_aws_crt.h * clang-format * Use a unity build to make PHP do the right thing with our lib

SamRemis · 2021-05-21T20:55:55Z

tests/SigningTest.php

        ]);
        $http_request = new Request('GET', '/', [], ['Host' => 'example.amazonaws.com']);
        $this->assertNotNull($http_request, "Unable to create HttpRequest for signing");
        $signable = Signable::fromHttpRequest($http_request);
        $this->assertNotNull($signable, "Unable to create signable from HttpRequest");

-        Signing::signRequestAws($signable, $signing_config, function() {
-
+        Signing::signRequestAws($signable, $signing_config, function($signing_result, $error_code) use (&$http_request) {


this looks like more than 80 characters and if it is should be split into more lines

SamRemis · 2021-05-21T20:56:03Z

tests/SigningTest.php

        });
+
+        $headers = $http_request->headers();
+        $this->assertEquals('AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;x-amz-date, Signature=5fa00fa31553b73ebf1942676e86291e8372ff2a2260956d9b8aae1d763fbf31', $headers->get('Authorization'));


same with above

In order to shrink this, I would have to heredoc it, and then strip newlines from it. That seems a bit counter to simplicity, eh?

SamRemis · 2021-06-02T16:53:02Z

Please add a method to expose the following line:
if (!extension_loaded('awscrt')) {}

This will reduce code duplication and enable us to throw an error when the extension isn't loaded and a feature requires sigv4a

justinboswell · 2021-06-02T22:06:20Z

Please add a method to expose the following line:
if (!extension_loaded('awscrt')) {}

This will reduce code duplication and enable us to throw an error when the extension isn't loaded and a feature requires sigv4a

See CRT::isAvailable() and CRT::isLoaded()

src/AWS/CRT/Auth/Signing.php

SamRemis · 2021-06-07T15:14:43Z

src/AWS/CRT/Auth/SigningConfigAWS.php

@@ -23,6 +23,7 @@ public static function defaults() {
            'signed_body_value' => null,
            'signed_body_header_type' => SignedBodyHeaderType::NONE,
            'expiration_in_seconds' => 0,
+            'date' => time()


Since this is a unix date, it could be named unixDate or something depending on how it is used to make it clear it has the exact timestamp?

so while it is a timestamp, it will be quantized to the year/month/day, so it is a date. In the SigV4 docs:

Note that the date used in the hashing process is in the format YYYYMMDD (for example, 20150830), and does not include the time.

SamRemis · 2021-06-07T15:18:47Z

src/AWS/CRT/Auth/SigningConfigAWS.php

@@ -34,7 +35,9 @@ public function __construct(array $options = []) {
        $sc = $this->acquire(self::$crt->signing_config_aws_new());
        self::$crt->signing_config_aws_set_algorithm($sc, $options->algorithm->asInt());
        self::$crt->signing_config_aws_set_signature_type($sc, $options->signature_type->asInt());
-        self::$crt->signing_config_aws_set_credentials_provider($sc, $options->credentials_provider->asObject());
+        if ($credentials_provider = $options->credentials_provider->asObject()) {
+            self::$crt->signing_config_aws_set_credentials_provider($sc, $credentials_provider->native);


92 characters so it should be split among a few lines:
self::$crt->signing_config_aws_set_credentials_provider(\n \t$sc,\n \t$credentials_provider->native\n );

We need to pick a linter. This is going to kill me. This is what tools are for :)

Fixed it anyway

SamRemis · 2021-06-07T15:20:07Z

src/AWS/CRT/Auth/SigningResult.php

+    }
+
+    function __destruct() {
+        // No release necessary, SigningResults are transient


Why is this comment here? And if it's not necessary, why do you do it on the next line?

Fixed, this was indeed confusing as hell

src/AWS/CRT/CRT.php

SamRemis

Hope you don't mind if I just run the whole thing through a PHP linter at some point in the future. Figure it'll be easier that way

justinboswell · 2021-06-08T00:19:08Z

Hope you don't mind if I just run the whole thing through a PHP linter at some point in the future. Figure it'll be easier that way

I will be overjoyed! Then tell me what it is so I can use it to verify!

* Added CI runs for all versions of PHP * PHP >= 7.1 is required for arginfo generation * Allow PHP 5.5 to compile * Added extension API detection for every minor version we support * Pinned version of phpunit for ancient php 5.5 * Added special php 5.5 CI * Updated to builder v0.8.18 * Added shim for expectException

Justin Boswell added 2 commits May 18, 2021 17:35

Updated to get latest aws-crt-ffi

332cf40

Added implementation for signing_result

4e33b9e

justinboswell requested review from SamRemis and a team May 19, 2021 00:36

Justin Boswell added 14 commits May 18, 2021 17:37

clang-format

fb1c7d8

Updated aws-crt-ffi to get mutex API

77ae80e

Fixed up signing code path, wip (fixing callback invoke)

7d8fd52

Updates to get promise API and fixes

89119a7

Updated aws-crt-ffi to get promise updates

9f971a2

Added main thread queue and callback queueing to signing process

1f42371

Updated to get support for date on signing_config_aws

a097f32

added date support to SigningConfigAWS, fixed up reference issues on …

c211099

…http message signing

Fixed tyop

34b0cb7

clang-format, fixed clang-format's wagon re: headers

ed89697

Self CR

1c8688d

Cleaned up thread queue semantics

81f89eb

Added assert for thread homing for aws_php_thread_queue_drain

5a5e38d

Improved comment on signing queueing chain

d816265

justinboswell marked this pull request as ready for review May 19, 2021 21:56

Justin Boswell added 3 commits May 19, 2021 14:58

Fixed header order, god damn you clang-format

54724e9

God dammit you fuckers

ec09c56

Fixed php5 function invoke

389ad83

justinboswell changed the title ~~Implemented Sigv4~~ Implemented async callbacks to PHP regardless of thread model, Sigv4 signing May 20, 2021

justinboswell mentioned this pull request May 21, 2021

Added support for signing result, promise API awslabs/aws-crt-ffi#9

Merged

Broke C code into separate files, moved common stuff to php_aws_crt.h (…

c3c1416

…#25) * Broke C code into separate files, moved common stuff to php_aws_crt.h * clang-format * Use a unity build to make PHP do the right thing with our lib

SamRemis reviewed May 21, 2021

View reviewed changes

Justin Boswell added 2 commits June 2, 2021 11:38

Removed use of FFI's mutex/thread APIs before they're deleted

6d6833f

Updated aws-crt-ffi to v0.1.3 to get sigv4 changes

24511e2

Justin Boswell added 3 commits June 2, 2021 11:55

Added CRT::isAvailable for safe runtime detection of CRT

4b6874c

Updated aws-crt-ffi, updated promise API usage

e174c76

Fixed up test resource safety, added tests to prove it

a2e9461

Formatting feedback

f8ca268

SamRemis reviewed Jun 7, 2021

View reviewed changes

Formatting/PR feedback

4043ee3

SamRemis approved these changes Jun 8, 2021

View reviewed changes

justinboswell enabled auto-merge (squash) June 8, 2021 00:31

justinboswell merged commit 7d47316 into main Jun 8, 2021

justinboswell deleted the sigv4 branch June 8, 2021 00:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented async callbacks to PHP regardless of thread model, Sigv4 signing #24

Implemented async callbacks to PHP regardless of thread model, Sigv4 signing #24

justinboswell commented May 19, 2021 •

edited

Loading

SamRemis May 21, 2021

SamRemis May 21, 2021

justinboswell Jun 2, 2021

SamRemis commented Jun 2, 2021

justinboswell commented Jun 2, 2021

SamRemis Jun 7, 2021

justinboswell Jun 7, 2021

SamRemis Jun 7, 2021

justinboswell Jun 7, 2021

justinboswell Jun 7, 2021

SamRemis Jun 7, 2021

justinboswell Jun 7, 2021

SamRemis left a comment

justinboswell commented Jun 8, 2021

Implemented async callbacks to PHP regardless of thread model, Sigv4 signing #24

Implemented async callbacks to PHP regardless of thread model, Sigv4 signing #24

Conversation

justinboswell commented May 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SamRemis commented Jun 2, 2021

justinboswell commented Jun 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SamRemis left a comment

Choose a reason for hiding this comment

justinboswell commented Jun 8, 2021

justinboswell commented May 19, 2021 •

edited

Loading