Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

How to handle client POST requests #251

Closed
tripodsan opened this issue Apr 11, 2019 · 9 comments
Closed

How to handle client POST requests #251

tripodsan opened this issue Apr 11, 2019 · 9 comments
Labels
question Further information is requested

Comments

@tripodsan
Copy link
Contributor

(copied over from #235)

In general, we have to decide how (and if) we want to handle POST requests. either application/json bodies, application/x-www-form-urlencoded and multipart/form-data.

Fastly

currently, any POST requests to fastly are not allowed. the internal action delegation makes heavy use of request parameters, which are manifested in the actions params eg:

  "params": {
    "__ow_path": "",
    "extension": "",
    "owner": "tripodsan",
    "params": "",
    "path": "/index.md",
    "ref": "master",
    "repo": "hlxtest",
    "rootPath": "",
    "selector": "",
    "strain": "default"
  },

So even if we allow POST requests, we have to ensure that they don't override those params. so any handling of form-param, or JSON bodies MUST NOT override any action param.

Openwhisk

Currently, we deploy the actions as normal web actions. one cool feature of them is that:

The __ow_body property is present either when handling "raw" HTTP requests, or when the HTTP request entity is not a JSON object or form data. Web actions otherwise receive query and body parameters as first class properties in the action arguments with body parameters taking precedence over query parameters, which in turn take precedence over action and package parameters.

As mentioned above, this behaviour is not desired, as it conflicts with the way we invoke the actions via fastly.

as a consequence, we SHOULD deploy the script as raw actions with --web=raw. this would imply that that pipeline needs to handle the __ow_query parsing.

Pipeline

Given the pre-conditions above, using raw web actions would imply to handle the __ow_query in the pipeline (openwhisk.js) and decode them into the action.params.

  • for a POST request with content-type application/x-www-form-urlencoded, we SHOULD decode them into the context.request.params property, since they specify client input.

  • for a POST request with content-type multipart/form-data, the pipeline COULD handle the form data and populate context.request.params, or maybe a context.request.formdata to provide access to the binaries in some form.

  • for a POST request with content-type application/json, the pipeline SHOULD decode the json body and populate content.request.body

  • for a POST request with any other content-type, the pipeline SHOULD keep the raw data in content.request.body as Buffer. this way, application code can easily check with Buffer.isBuffer() for raw data.

Simulator

The helix simulator not only emulates fastly, but also the openwhisk gateway. so any changes to the above, need to be reflected in the simulator:

  • use __ow_query instead of populating actions params
  • create __ow_body with base64 encoded request.body for POST requests
@tripodsan tripodsan added the question Further information is requested label Apr 11, 2019
@trieloff
Copy link
Contributor

How important is POST support?

@tripodsan
Copy link
Contributor Author

tripodsan commented Apr 15, 2019

How important is POST support?

so far we didn't have any requests for it...

(giving some context: I wrote this originally in #235, when I thought we need to change the entire POST processing in order to support the live-review case. after I finished writing, I realized that the live-review POSTs directly to the action; so those changes would not be needed right now.)

@trieloff
Copy link
Contributor

I think for the time being, we can use POSTs straight to the action. I'd only enable POSTs for cgi-bin anyway, so we might never need this here.

@rofe
Copy link
Contributor

rofe commented Apr 16, 2019

I see 2 challenges for live previewing: POST and CORS.

The Chrome extension POC @rofe/helix-markdown-preview currently only supports POST to localhost where CORS isn’t an issue.

POST to the action was used in the POC at @kptdobe/helix-preview which was done for the last hackathon. It’s a possible solution, but doesn’t need to be the final architecture yet for live previewing during authoring. Runtime does allow CORS by design, but using the rather complicated action URL could become impractical, unless we completely abstract it away from the end user.

Maybe an authoring client like the Chrome extension could auto-extract the action URL using a request with x-debug header first, and then POST there?

If we were to allow POSTs via Fastly - which we eventually may have to in order to support customer use cases like form submits - we’d also need to think when to send the proper CORS headers on both OPTIONS and POST requests, either all the time (probably not recommended) or if in certain config is present...

In any case, I think we’ll have to revisit the fact that the html pipe returns whatever body it gets sent... A safer solution as suggested by @lkrapf would be to use a dedicated pipe for live previewing, which would always return the html wrapped in json, and with CORS headers.

@tripodsan
Copy link
Contributor Author

I think the whole how-do-I-initialize-the-live-preview question should be discussed somewhere else (e.g: getting the correct action AND content repository tuple)

A safer solution as suggested by @lkrapf would be to use a dedicated pipe for live previewing, which would always return the html wrapped in json, and with CORS headers.

agreed.

@trieloff
Copy link
Contributor

For live previews in particular, we could also build a new request type in VCL that still does the action resolution but simply passes through the entire POST body. This would be almost equivalent to calling the action directly, but save us from URL strangeness.

@tripodsan
Copy link
Contributor Author

tripodsan commented Apr 18, 2019

build a new request type in VCL

github uses media types via theaccept header, which is IMO a good idea. for example

curl -X POST -H "Accept: application/vnd.helix.preview+json"  -d @newindex.md https://www.myproject.com/docs/general/index.md

> 200 OK

{
  "content": "<doctype html>.... ",
}

@trieloff
Copy link
Contributor

The X-Request-Type in our VCL is internal only, and it specifies which backend service should be used, see

https://github.com/adobe/helix-publish/blob/3708b698c951f8335e75518c7f853cfda12aeca0/layouts/fastly/helix.vcl#L740-L752

Based on the Accept header, we could set that.

@kptdobe
Copy link
Contributor

kptdobe commented Aug 28, 2019

Closed during backlog grooming session during hackathon. Reopen if needed or open a new one.

@kptdobe kptdobe closed this as completed Aug 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants