requestbody: Add replace for optional body replacement

[AdrienPensart]

Hello,

here is a need I have, I use caddy as a reverse proxy to EdgeDB endpoint, and declare custom routes to query Edgedb.
The endpoint act as an alias for my query, so no query appears in the javascript on client side.

:80 {
    file_server browse
    root * hyperedge
    handle /artists {
        method POST
        request_body {
            replace `{"query": "select artists {*}"}`
        }
        request_header Content-Type application/json
        rewrite * /db/edgedb/edgeql
        reverse_proxy http://localhost:10701
    }
}

Tell if it's a good idea or if the name of the directive should be something else.

[CLAassistant]

All committers have signed the CLA.

[francislavoie]

This is interesting, but a bit strange of a feature. I think we'll need to do a bit of thinking about how we want to approach this. It might take us a bit longer to decide on this one.

Thanks for the contribution and idea!

[mholt]

Oh, so this actually completely drops the upstream response body and replaces it with a new one entirely, not performing replacements in the response body.

Should r.Body be closed before it is set to a new value?

Like Francis said, this is a bit unusual and I'm not sure what the implications will be.

I guess it's a very simple change. (We're on feature freeze until after 2.8, so even if we merge this in, it won't be released until 2.9.)

Why is this needed instead of a simple respond directive?

[mholt]

Would need a helpful godoc comment on this field.

[steffenbusch]

Hi @mholt

Oh, so this actually completely drops the upstream response body and replaces it with a new one entirely, not performing replacements in the response body.

I think it's only about the Request body.

Today, I have been playing with Oracle REST Data Services / ORDS where you can POST sql statements to a service endpoint of ORDS such as:

$ cat simple_query.json
{ "statementText":"SELECT TO_DATE('01-01-1976','dd-mm-yyyy') FROM dual;"}
$ curl -i -X POST --user DEMO:demo --data-binary "@simple_query.json" -H "Content-Type: application/json" -k http://localhost:8088/ords/demo/_/sql

Having a reverse_proxy to http://localhost:8088/, you could hide the SQL from users/clients with the configration of @AdrienPensart and his proposal of request_body such as:

:443 {
    handle /1976 {
        method POST
        request_body {
            replace `{ "statementText":"SELECT TO_DATE('01-01-1976','dd-mm-yyyy') FROM dual;"}`
        }
        request_header Content-Type application/json
        rewrite * /ords/demo/_/sql
        reverse_proxy http://localhost:8088 {
            header_up Authorization "Basic REVNTzpkZW1v"
        }
    }
}

I thought I just add this use case here where this request_body + replace could be beneficial

[francislavoie]

Yeah - FWIW I thought this was about the response body at first as well, but this is definitely about the request body @mholt. Not the same as https://github.com/caddyserver/replace-response, entirely unrelated in fact.

In terms of Caddyfile API, I'm considering whether we should make it the first argument to request_body so you don't need to make a block to use replace (though we can keep replace as a long-form for people who want it). See https://caddyserver.com/docs/caddyfile/directives/request_body it could look like:

request_body [<matcher>] [<replacement>] {
	replace  <replacement>
	max_size <value>
}

[mholt]

Oh, I see -- thanks for pointing that out.

I like Francis' suggestion to make it work inline, i.e.: request_body "contents here" -- also, I want to give second thought to the term "replace" which, for me, makes me think of performing replacements within the request body, not replacing the whole request body. But I can see how both could be accurate. Hmm.

[mholt]

If we're completely replacing the request body, we miiiight want to call r.Body.Close() before re-assigning a new body. Yes the std lib will close it for us when the request is over, but would closing it early (and releasing those resources) cause any problems?

Also, I wonder if we should add placeholder support and expand those when doing the body replacement.