Add the ability to provide parameters via urlencoded form

[jefferai]

Prereq for some things coming later (maybe)

Makes using curl way nicer

Additionally, adds the ability to disable TLS for test clusters

[kalafut]

This parsing will not have the max_request_size limiter imposed. Is that something you'd want to put at the start, applicable to both parsing paths?

[jefferai]

Yeah, it's true. I guess I should put it behind that, it just makes things a bit more complicated.

[jefferai]

Fixed

[kalafut]

I believe this will cause surprises for curl and possibly other users. The default content-type curl for JSON data will be application/x-www-form-urlencoded. I confirmed that this command no longer works:

curl \
    --header "X-Vault-Token: root" \
    --request POST \
    --data @payload.json \
    http://127.0.0.1:8200/v1/secret/data/my-secret
{"errors":["no data provided"]}

Instead you have to do:

curl \
    --header "X-Vault-Token: root" --header "Content-Type: application/json"
    --request POST \
    --data @payload.json \
    http://127.0.0.1:8200/v1/secret/data/my-secret

In the first case, the data is getting parsed as a form with key="[payload.json contents]" value="".

[jefferai]

That may be a totally acceptable tradeoff. You can use --data-binary instead with curl which should work.

[briankassouf]

@jefferai I worry this change would break backwards compatibility for scripts/applications, thoughts?

[jefferai]

Sure, which is why we should probably not merge this yet.

[jefferai]

Stupid question. What if we just check the first character to see if it's {? Things that take bodies that are JSON are always JSON objects, AFAIK. So if the first byte is that, we expect JSON, otherwise, trust whatever content-type.

[kalafut]

I think an initial classification of "this looks like it's intended to be JSON, so assume it is" could work. The criteria should probably be something like (as regex): ^\s*[[{]

[jefferai]

All input to Vault is specified as part of a map -- as far as I can recall -- so I think really it would be just checking whether the first rune is {.

[kalafut]

Yeah, an array input doesn't seem too likely. I think leading whitespace is still something to be considered, however. It's valid JSON and could easily be present, whether via some client or just an errant leading space in a script.

[chrishoffman]

Closing this for now. We can reopen or create a new PR if we still want to go this route.