ZF2-268: AmazonS3 service broken

Description


$s = new \Zend\Service\Amazon\S3\S3($amazonKey,$amazonSecret);
$s->getBuckets();

result: bq. Warning: Missing argument 2 for Zend\Http\Client::setAuth(), called in /vendor/ZendFramework/library/Zend/Service/Amazon/S3/S3.php on line 624 and defined in /vendor/ZendFramework/library/Zend/Http/Client.php on line 635

And exception: bq. The username and the password cannot be empty

Because


Zend\Service\Amazon\S3\S3
624 $client->setAuth(false);

Zend\Http\Client
635 public function setAuth($user, $password, $type = self::AUTH_BASIC)

Please check this service.

Comments

$client->setAuth(false); means disable http auth, but Zend\Http\Client does not support it yet, I create a patch to support it.

support setAuth(false)

Thank you, Simon Liu! But apparently, this class Zend\Service\Amazon\S3 has not yet upgraded to ZF2.

Because the line 651


651 $response = $client->request($method);

But new class Zend\Http\Client don't have method request(), although the old class Zend_Http_Client have request() method.

Can you finish this S3 class? Or when it is planned to be done?

Thank you!

We have to refactor the Zend\Service\Amazon component with the new Zend\Http\Client. We will do that in the next beta (not beta4).

Enrico, thanks for the info!

Can we have confirmation this will be fixed in beta4, this prevents usage of the S3 component and shouldn't be included if not working at all.

This line appears to be useless because two rows above in S3::_makeRequest() there is $client->resetParameters():

622 $client->resetParameters(); 623 $client->setUri($endpoint); 624 $client->setAuth(false);

resetParameters already resets the Auth.

Closely related to this bug, on the next line there's

    // Work around buglet in HTTP client - it doesn't clean headers
    // Remove when ZHC is fixed
    $client->setHeaders(array('Content-MD5'              => null,
                              'Expect'                   => null,
                              'Range'                    => null,
                              'x-amz-acl'                => null,
                              'x-amz-copy-source'        => null,
                              'x-amz-metadata-directive' => null));

    $client->setHeaders($headers);

The first setHeaders raises an exception. setHeader now replaces the old headers, so it is safe to take away the first setHeader and just leave the $client->setHeaders($headers);

If you think that you can fix this submit a patch to Github (https://github.com/zendframework/zf2/)

Also could be very interesting provide a unit test for this.

Sorry, but one small bug after another, I believe I fixed it.

https://github.com/zendframework/zf2/pull/1173

I've never written a unit test for a http-based service. To your knowledge, is there other examples that could be used as a starting point?

Any of this https://github.com/zendframework/zf2/… can help you.

For example, if you want add the test to https://github.com/zendframework/zf2/…

then create a function like this 'public function testXXXXX()' and put in the body something like this:

$s3 = new S3(); ...

$actual = s3->getXXX(); $expects = 'Foo';

$this->assertEqual($expects, $actual);

As you can see the first part is test the function, then you storage the result in a var and finally you test if the result is the same of you expects.

This can help you http://phpunit.de/manual/3.6/…

I tested it for a couple of days and put the relevant changes to pull #1194 https://github.com/zendframework/zf2/pull/1194

Thank you Maks for the info on unit test. I know more or less how to write a unit test, but I don't know what's the standard for writing a unit test based on an external service: unless you create a fake in-out stream (is it possible? isn't it crazy?), or anyway a test-repo on S3 or a public sandbox by the service owner, results will be always variable depending on the status of the external service.

However, it appears that there's already a neatly written test for the S3 service https://github.com/zendframework/zf2/… . At a first glance it should work, but I haven't run it yet.

To avoid unexpected responses from a external service you should create a Mockup (http://phpunit.de/manual/current/…) Is a little more complex to learn how to use but I tell you the idea behind this.

I will guess the following scenario (I don't know the internals of S3) You request an authorization and S3 transform your request in a Http response

User/Pass <=> S3::auth($parameters) <=> Zend\Http\Client::send($request)

The goal here is that when Http\Client receive $request you already know if S3::auth() do his works correctly.

Then we want convert Http\Client in a mockup object and then you setup something as following (pseudocode):

$mock = createMockup(Zend\Http\Client) $mock -> method(send) $mock -> expects($request = http://amazon/?u=User&p=Pass) $mock -> return(the expected Http result to use inside of S3)

Now we need inject our mockup inside of S3 class. We need something like S3::setHttpClient($mock)

Then when you execute the test you can be sure about the functionality.

Of course we are talking about Unit Tests where the scope is limited to test the code of the function. Other tests are Functional Tests where we don't use mockups and we do direct connections to AWS servers (OnlineTests.php)

You can enter to the IRC if you need more help. #zftalk.2 in freenode.net

I wrote a small test to verify the responses for getBuckets() this test not verify the auth issue described above but could give you an example about how works the mockups.

https://github.com/zendframework/zf2/pull/1203

After upgrading from beta4 to beta5 the S3 service stopped working: all the operations (for example, getBuckets) leads to an error in the response of the service: ‘The request signature we calculated does not match the signature you provided. Check your key and signing method.’

In beta4 it works.

I’ve checked the difference between beta4 and beta5 and found the calculating of signature was changed in addSignature method of S3 class.

In beta4 it was:


$signature = base64_encode(Hmac::compute($this->_getSecretKey(), 'sha1', utf8_encode($sig_str), Hmac::OUTPUT_BINARY));

In beta5 it is:


$signature = base64_encode(Hmac::compute($this->_getSecretKey(), 'sha1', utf8_encode($sig_str), true));

Could you please explain why the last argument of Hmac::compute was changed in this way? Or if it was made by mistake could you roll back this?

Hi [~boristm] could you open your issue in a new one?