aws / aws-sdk-php

Official repository of the AWS SDK for PHP (@awsforphp)
http://aws.amazon.com/sdkforphp
Apache License 2.0
6.03k stars 1.22k forks source link

Message::fromRawPostData incompatible with raw POST data #533

Closed gummoe closed 9 years ago

gummoe commented 9 years ago

The Problem: The fromRawPostData() function does not appear to work in any situation at the moment.

Diagnosis: The Message::fromRawPostData() function relies on json_decode(file_get_contents('php://input'), true) to return an array that is then passed to the Message::fromArray constructor. Unfortunately, Amazon SNS POST requests have a Content-Type of text/plain, which makes it impossible for the json_decode call to return anything other than null. This results in an exception when fromRawPostData() calls the fromArray constructor.

Three possible solutions:

  1. Change the SNS POST request Content-Type to 'application/json' or some other acceptable Content-Type
  2. Add a private POST body-parsing function to the Message class
  3. Leave it up to users to provide a well-formed array to the fromAray() constructor and remove the fromRawPostData() function

I'd be happy to contribute any work required for solutions 1 or 2!

mtdowling commented 9 years ago

Just to make sure we're on the same page: the class you're talking about is https://github.com/aws/aws-sdk-php/blob/master/src/Aws/Sns/MessageValidator/Message.php, which is used to validate POST requests sent from SNS to your server. Correct?

Unfortunately, Amazon SNS POST requests have a Content-Type of text/plain, which makes it impossible for the json_decode call to return anything other than null.

The fromRawPostData() function doesn't check the content-type of the HTTP request that it received. It just takes the raw body of the HTTP request that was received and tries to JSON parse it. The content-type sent from the service shouldn't matter.

  1. Change the SNS POST request Content-Type to 'application/json' or some other acceptable Content-Type

The POST comes from the service, so we can't change it from the client.

  1. Add a private POST body-parsing function to the Message class

Are you saying that the POST body is not actually JSON data?

mtdowling commented 9 years ago

Do you have any more information on this?

gummoe commented 9 years ago

Sorry for my delayed response, and thank you for helping me clarify my thoughts!

You are correct that this is the class in question: https://github.com/aws/aws-sdk-php/blob/master/src/Aws/Sns/MessageValidator/Message.php

You are also correct that the fromRawPostData() function does NOT check the Content-Type of the HTTP request.

However...I may be wrong, but the value obtained from the file_get_contents('php://input') returns the entire raw request. An example of a notification from SNS is included below:

POST /app_dev.php/event/test/test1 HTTP/1.1
Host: [sanitized] 
User-Agent: Amazon Simple Notification Service Agent
Content-Length: 1135
Accept-Encoding: gzip,deflate
Connection: close
Content-Type: text/plain; charset=UTF-8
X-Amz-Sns-Message-Id: [sanitized]
X-Amz-Sns-Message-Type: Notification
X-Amz-Sns-Subscription-Arn: [sanitized]
X-Amz-Sns-Topic-Arn: [sanitized]
X-Forwarded-Proto: https
X-Real-Ip: 72.21.217.131

{
"Type" : "Notification",
"MessageId" : "[sanitized]",
"TopicArn" : "[sanitized]",
"Subject" : "This is the subject",
"Message" : "[sanitized]",
"Timestamp" : "2015-04-16T17:37:05.278Z",
"SignatureVersion" : "1",
"Signature" : "[sanitized]",
"SigningCertURL" : "[sanitized]",
"UnsubscribeURL" : "[sanitized]"
}

The json_decode function expects the entire string (in this case, php://input) to be JSON-encoded, which the raw request is not. I agree with you that the content-type shouldn't matter, but it also seems to be the case that the fromRawPostData() function needs to extract the correctly formatted 'payload'-portion of the SNS request before calling json_decode. Happy to provide any additional information or tests that would be helpful!

mtdowling commented 9 years ago

Reading from php://input should just include the body of the received request: http://php.net/manual/en/wrappers.php.php#wrappers.php.input. Are you seeing a different behavior?

gummoe commented 9 years ago

Yup! php://input seems to return the entire request (including headers) in my environment.

mtdowling commented 9 years ago

What environment are you running in?

gummoe commented 9 years ago

Nothing fancy: PHP v5.3.29 Composer.json has AWS SDK set to: "aws/aws-sdk-php": "2.7.*" Hosting using MAMP PRO.

mtdowling commented 9 years ago

I just tested this on Amazon Linux, and the contents of php://input does not contain the headers.

<?php
echo file_get_contents('php://input');

Output of this running under Apache with PHP 5.6.8

curl -v --data 'testing' http://127.0.0.1/test.php
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 80 (#0)
> POST /test.php HTTP/1.1
> User-Agent: curl/7.40.0
> Host: 127.0.0.1
> Accept: */*
> Content-Length: 7
> Content-Type: application/x-www-form-urlencoded
> 
* upload completely sent off: 7 out of 7 bytes
< HTTP/1.1 200 OK
< Date: Wed, 22 Apr 2015 19:50:26 GMT
< Server: Apache/2.4.12 (Amazon) PHP/5.6.8
< X-Powered-By: PHP/5.6.8
< Content-Length: 7
< Content-Type: text/html; charset=UTF-8
< 
* Connection #0 to host 127.0.0.1 left intact

testing

I got the same result for PHP 5.3.29 and PHP 5.4.40.

You said that it "seems" to return the full request including headers. Can you write up a script to prove that and share the results with us? I want to make sure that this is the case even on Windows before we change anything.

mtdowling commented 9 years ago

@gummoe Were you able to confirm that php://input includes request headers? Based on my tests, the PHP documentation, and tests in the PHP git repo, it should not contain request headers.

If you look at some of the acceptance tests in php-src, you can see that the headers are not included:

mtdowling commented 9 years ago

I'm going to close this issue. If you are receiving headers in the contents of php://input, then it is likely an environment specific issue. If that is not the case, it would be helpful to have a reproducible test case that you can provide.

gummoe commented 9 years ago

@mtdowling - sorry for the delayed response and the confusion. I've been able to confirm that this was the result of a parsing error in my testing environment that was incorrectly placing request headers into the raw post body. Thank you for your patience and for excusing my usage of your time!