search

adlnet / lrs-conformance-test-suite

A NodeJS project that tests the MUST requirements of the xAPI Spec and is based on the ADL testing requirements repository. The test suite website can be found here: https://lrstest.adlnet.gov/. The adopters website can be found here: https://adopters.adlnet.gov/
https://adlnet.gov/projects/xapi/
MIT License
67 stars 42 forks source link

Tests send body with GET, incompatible with Amazon CloudFront #196

Closed bscSCORM closed 7 years ago

bscSCORM commented 7 years ago

Several tests send GET requests that include a content body, which causes Amazon CloudFront to return 403. These are all tests where we expect 400, so there wouldn't seem to be an interoperability penalty to allowing either 400 or 403. And Amazon is popular enough it seems like we should have good reason to send tests that are incompatible with CloudFront. Furthermore, a content body as part of a GET request is a very uncommon sort of request, so it seems likely that other proxy servers might also have an issue with it.

The tests are:

FAILED: An LRS rejects an alternate request syntax not issued as a POST

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: An LRS's Agent Profile Resource rejects a GET request without "agent" as a parameter with error code 400 Bad Request (multiplicity, Communication 2.6.s4.table1.row1, XAPI-00261)
[ 'Communication.md#2.6.s4.table1.row1' ]
RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: Should reject GET with "agent" with type 1

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: Should reject GET with "agent" with type true

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: Should reject GET with "agent" with type [object Object]

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: Should reject GET with "agent" with type not Actor

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: Should reject GET with "since" with type 1

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: Should reject GET with "since" with type true

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: Should reject GET with "since" with type [object Object]

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************
FAILED: Should reject GET with "since" with type not timestamp

RejectionError: expected 400 "Bad Request", got 403 "Forbidden"

***************************************

I submitted a sample request to Amazon to ask why they were behaving this way (note: this request doesn't simulate any particular test or valid request, it's just a quick request I threw together to reproduce the issue). The responded:

We likely return a 403 because we do not support a body in a GET request through CloudFront. CloudFront is choosing not to continue with the request in this case. From the spec it says that:

The presence of a message-body in a request is signaled by the inclusion of a Content-Length or Transfer-Encoding header field in the request's message-headers. A message-body MUST NOT be included in a request if the specification of the request method (section 5.1.1) does not allow sending an entity-body in requests. A server SHOULD read and forward a message-body on any request; if the request method does not include defined semantics for an entity-body, then the message-body SHOULD be ignored when handling the request.

Now it says specifically "SHOULD" but does not say "MUST" so this means that there is no specific requirement to behave one way or another. In your case, CloudFront chose the way they behave currently.

The correct way to do this is with a HEAD request see:

curl -i -v  https://cloud.scorm.com/tc/public/statements?limit=2 --data "param1=value1&param2=value2"

> POST /tc/public/statements?limit=2 HTTP/1.1
> Host: cloud.scorm.com
> User-Agent: curl/7.52.1
> Accept: */*
> Content-Length: 27
> Content-Type: application/x-www-form-urlencoded
>
* Connection state changed (MAX_CONCURRENT_STREAMS updated)!
* We are completely uploaded and fine
< HTTP/2 400
HTTP/2 400
< content-length: 113
content-length: 113
< date: Tue, 09 May 2017 21:24:14 GMT
date: Tue, 09 May 2017 21:24:14 GMT
< server: Apache
server: Apache
< x-xss-protection: 1; mode=block
x-xss-protection: 1; mode=block
< x-cache: Error from cloudfront
x-cache: Error from cloudfront
< via: 1.1 d98420743a69852491bbdea73f7680bd.cloudfront.net (CloudFront)
via: 1.1 d98420743a69852491bbdea73f7680bd.cloudfront.net (CloudFront)
< x-amz-cf-id: N5HlnAXHcBHyg_YRqhSecy2fDbLb-I-2wKeK01-M29RR272BCcSNWA==
x-amz-cf-id: N5HlnAXHcBHyg_YRqhSecy2fDbLb-I-2wKeK01-M29RR272BCcSNWA==

This sends the parameters correctly from what I can tell. Can you check that this is performing the operation successfully on your end with the correct data?
ljwolford commented 7 years ago

@bscSCORM - after looking at them closer, we saw that the tests should not have been sending anything in the body, and instead should have been sending the params in the query string. There was also an instance of having an extra invalid parameter being sent in to the profile endpoint. We will clean those up and they will be included in the next update instead of this one to ensure everything is working as designed. Thanks for bringing this to our attention!

bscSCORM commented 7 years ago

@ljwolford @cr8onski I verified that the current development branch (1a4495c9d895876cdece3037597206b2bcea53c5) works through CloudFront now