buildkite / feedback

Got feedback? Please let us know!
https://buildkite.com
25 stars 24 forks source link

Server accepts REST API requests with wrong content-type #335

Open philwo opened 6 years ago

philwo commented 6 years ago

Hi,

we recently had an interesting issue with our CI scripts, where Buildkite builds could not be triggered via the API, when the "message" field in the JSON request contained a percent character:

curl -X POST "https://api.buildkite.com/v2/organizations/bazel/pipelines/re2/builds" \
  -H "Authorization: Bearer $BUILDKITE_API_TOKEN" \
  -d '{
    "commit": "HEAD",
    "branch": "master",
    "message": "Testing all the %s strings"
  }'

The reason was that we didn't set the Content-Type header correctly, so curl used application/x-www-form-urlencoded by default. This (surprisingly) works fine, until you add a % character somewhere, because then the Buildkite backend tries to do URL decoding on the POST payload, which fails with a 500 Internal Server Error.

Adding this: -H "Content-Type: application/json" to the curl command-line fixed the issue and now the above request also works.

I think the right behavior would be for the server to reject API requests that do not set the Content-Type correctly to application/json, however this would be a breaking change so probably not desirable until you switch to API version v3.

It would probably be good to add the -H "Content-Type: application/json" flag to the API examples where you run curl to point out the fact that it's important to set this correctly. :) Alternatively, maybe it's possible to actually ignore the Content-Type header for incoming requests and do not try to URL decode the payload.

The docs actually state that this "should" work as the data encoding is assumed to be application/json, but as seen above this is not 100% the case:

The data encoding is assumed to be application/json. Unless explicitly stated you can not encode properties as www-form-urlencoded or multipart/form-data.