sillygod / cdp-cache

a caddy 2 proxy cache plugin
MIT License
119 stars 17 forks source link

Support HTTP POST requests #44

Closed corneliusludmann closed 3 years ago

corneliusludmann commented 3 years ago

We at Gitpod would like to use this great Caddy plugin for caching HTTP POST requests. Unfortunately, only GET and HEAD requests are supported yet. This PR adds the possibility to configure this plugin to cache HTTP POST requests as well.

This PR provides 2 commits:

For caching POST requests it's important to distinguish requests with different body contents. The second commit adds http.request.contentlength and http.request.bodyhash. For the body hash it's important that the cache key is calculated before the body has been read (before it's empty). Therefore the key is passed to fetchUpstream.

We would be happy if you would accept this change and thus expand the possibilities of the use of this plugin. Please let me know if you need anything else from me.

sillygod commented 3 years ago

@corneliusludmann hi,

This is an interesting design. However, I don't know whether it's appropriate to cache the HTTP request with the post method or not.

I found the part of rfc

Responses to this method are not cacheable unless the response includes appropriate Cache-Control or Expires header fields. However, the 303 (See Other) response can be used to direct the user agent to retrieve a cacheable resource.

It seems like the post method can be cached under the specified HTTP headers.

well, another rfc 7231 mentions

this specification defines GET, HEAD, and POST as cacheable, although the overwhelming majority of cache implementations only support GET and HEAD.

So I think it's ok to add this feature.

I will take some time to review this one. Thanks for this great work!