magnetikonline / edgy Goto Github PK

Currently, it looks like edgy verifies the result value of a ViewerRequest handler execution by delegating to lib.payloadVerifyRequest() via its ViewerRequest._payloadVerify method implementation. But this does not support response objects coming from viewer request functions (i.e. a short-circuit response, without connecting to the origin).

A viewer request function could yield a response object result, not always a request object result.

When a viewer request handler yields a response object (one with a status field), then lib.payloadVerifyRequest() will fail, and it will cause vReq.execute() to fail.

Example:

const edgy = require('@magnetikonline/edgy');

async function myTest() {
  const vReq = new edgy.ViewerRequest()
  vReq
    .setClientIp('1.2.3.4')
    .setHttpMethod('PUT')
    .setUri('/path/to/api/route')
    .addRequestHttpHeader('X-Fancy-Header','apples')

  const resp = await vReq.execute(
    // Fails request verification because the yielded payload is a response object:
    async function(event) {
      return { status: '302', statusDescription: 'Redirect', headers: {} }
    }
  )
}

Result:

Error: payload property [clientIp] not found
 ❯ payloadPropertyExists node_modules/@magnetikonline/edgy/lib.js:636:8
 ❯ payloadPropertyExistsString node_modules/@magnetikonline/edgy/lib.js:649:2
 ❯ Object.payloadVerifyRequest node_modules/@magnetikonline/edgy/lib.js:420:2
 ❯ ViewerRequest._payloadVerify node_modules/@magnetikonline/edgy/main.js:12:7
 ❯ ViewerRequest.execute node_modules/@magnetikonline/edgy/lib.js:140:8
 ❯ tests/index.test.js:291:20
    289|     .addRequestHttpHeader('X-Fancy-Header','apples')
    290| 
    291|   const response = await vReq.execute(
       |                    ^
    292|     // Fails request verification because the yielded payload is a response object:
    293|     async function(event) {

A workaround:

class ViewerRequest extends edgy.ViewerRequest {
  _payloadVerify(payload) {
     if ('status' in payload) { // Verify a response object result from the viewer request handler:
      edgyLib.payloadVerifyResponse(payload)
    } else { // Verify a request object result from the viewer request handler:
      edgyLib.payloadVerifyRequest(payload)
    }
  }
}

Aside: response object verification requires a headers field, but only a status field is strictly required.

The same problem may be true of OriginRequest too.

Could ViewerRequest and OriginRequest classes support direct response object results?

Hello, I've started playing with the library to set up full tests of our Lambda@Edge functions. Ideally, I'd like to chain together the 4 functions to replicate and test our flow in an offline context. We use Lambda@Edge for split testing across our sites so we modify and overwrite some of the event values.

When testing the edgy.OriginRequest, we noticed the isValidPath logic does not match the real implementation, and the AWS documentation is a bit misleading. In the Lambda Edge event structure docs on path it says:

The directory path at the origin where the request should locate content. The path should start with a forward slash (/) but shouldn't end with one

However what is missing from this that is in the Cloudfront Developer Guide. Where it starts with:

If you want CloudFront to request your content from a directory in your origin

Therefore I believe that the default value for telling cloudfront not to grab content from a different directory is just an empty string, instead of /. I tested this with a live cloudfront distribution, and if you pass a /, it will raise an error while an empty path or a "valid" path (something like /<env>/<version>) will work as expected.

I have updated this function locally to continue working through implementing testing and will hold any PRs in case of other fixes