Request body check fails w/ JSON body and wrong Content-Type header

[augensalat]

(Maybe some sort of duplicate of #76?)

use Mojolicious::Lite;

post "/echo" => sub {
  my $c = shift->openapi->valid_input or return;
  my $data = {body => $c->req->json};

  $c->render(openapi => $data);
}, "echo";

plugin OpenAPI => {url => "data:///openapi.yml", schema => "v3"};
app->start;

__DATA__
@@ openapi.yml
openapi: 3.0.0
info:
  version: "0.8"
  title: Echo Service
paths:
  /echo:
    post:
      x-mojo-name: echo
      requestBody:
        content:
          application/json:
            schema:
              type: object
              required:
                - id
                - name
              properties:
                id:
                  type: integer
                name:
                  type: string
      responses:
        "200":
          description: Echo response
          content:
            application/json:
              schema:
                type: object
servers:
  - url: /api

Start daemon and curl a request (keep an eye on the request Content-Type):

$ curl -v -d '{"id":"1"}' http://127.0.0.1:3000/api/echo
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 3000 (#0)
> POST /api/echo HTTP/1.1
> Host: 127.0.0.1:3000
> User-Agent: curl/7.64.1
> Accept: */*
> Content-Type: application/x-www-form-urlencoded
> Content-Length: 10
>
* upload completely sent off: 10 out of 10 bytes
< HTTP/1.1 200 OK
< Date: Wed, 14 Apr 2021 19:11:52 GMT
< Server: Mojolicious (Perl)
< Content-Type: application/json;charset=UTF-8
< Content-Length: 19
<
* Connection #0 to host 127.0.0.1 left intact
{"body":{"id":"1"}}

This is strange since the daemon happily processes the request body as JSON - seems the Content-Type: application/x-www-form-urlencoded is ignored. More severe: The input gets accepted even though the required body parameter name is missing!

You get the same wrong behavior with an explicitly wrong Content-Type header, i.e.

curl -v -d '{"id":"1"}' -H 'Content-Type: text/xml' http://127.0.0.1:3000/api/echo

However input validation works as expected when request comes with correct Content-Type:

$ curl -v -d '{"id":"1"}' -H 'Content-Type: application/json' http://127.0.0.1:3000/api/echo
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 3000 (#0)
> POST /api/echo HTTP/1.1
> Host: 127.0.0.1:3000
> User-Agent: curl/7.64.1
> Accept: */*
> Content-Type: application/json
> Content-Length: 10
>
* upload completely sent off: 10 out of 10 bytes
< HTTP/1.1 400 Bad Request
< Content-Length: 79
< Date: Wed, 14 Apr 2021 19:28:20 GMT
< Content-Type: application/json;charset=UTF-8
< Server: Mojolicious (Perl)
<
* Connection #0 to host 127.0.0.1 left intact
{"errors":[{"message":"Missing property.","path":"\/body\/name"}],"status":400}

and also without a Content-Type:

curl -v -d '{"id":"1"}' -H 'Content-Type:' http://127.0.0.1:3000/api/echo

Final note: Swagger/OpenAPI2 seems to do it right. Here is my v2 spec:

{
  "swagger" : "2.0",
  "info" : { "version": "0.8", "title" : "Echo Service" },
  "schemes" : [ "http" ],
  "basePath" : "/api",
  "paths" : {
    "/echo" : {
      "post" : {
        "x-mojo-name" : "echo",
        "consumes": [
          "application/json"
        ],
        "produces": [
          "application/json"
        ],
        "parameters" : [
          {
            "in": "body",
            "name": "body",
            "schema": {
              "type" : "object",
              "required": ["id", "name"],
              "properties": {
                "id": {
                  "type": "integer"
                },
                "name": {
                  "type": "string"
                }
              }
            }
          }
        ],
        "responses" : {
          "200": {
            "description": "Echo response",
            "schema": { "type": "object" }
          }
        }
      }
    }
  }
}

Money quote: A malicious client can dodge input validation for OpenAPI3 spec by sending a non-json Content-Type header.

[augensalat]

IMHO the correct response to a request header Content-Type that doesn't match any of the content specifications in the OpenAPI specification file should be a 415 Unsupported Media Type.

[xurc]

Same issue with OpenAPI v3 spec. More information below.

requires 'Mojolicious', '== 9.17';
requires 'JSON::Validator', '== 4.17';
requires 'Mojolicious::Plugin::OpenAPI', '== 4.03';

Spec has body required: true

        "requestBody": {
          "required": true,
          "content": {
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/UserFull"
              }
            }
          }
        }

my $c = shift->openapi->valid_input or return;

Work correctly only with exact header

Content-Type: application/json

But unvalidated body goes to controller code even with:

Content-Type: application/json;charset=UTF-8

Charset sent with almost any client browser.

[jhthorsen]

@augensalat: It would be so much easier for yourself and me if you learned how to write unit tests, instead of using curl.

Either way, this is fixed in json-validator, and will be released later today.

[christopherraa]

Is this the "correct" behaviour for v2 as well? Or v3 even? I've been trying to figure out if content type should be validated against consumes and produces in v2, but have found no evidence to suggest that it should be so. To be fair: I have also found no evidence that it shouldn't be validation here. However it looks to me based on discussions in the OAS working group that those attributes were meant and were used in a more informational capacity, and thus is not something that should be validated against.

Anyway: the change to enforce validation is actually a breaking change in that if you have clients that don't send content type presently and the API is updated with the latest version of JSON::Validator then those clients will suddenly have previously working requests be rejected. Is this a change that is suitable for a minor release?

We don't have to look far to find clients that don't send a default content type ($t->post_ok('/foo' => $data)). When looking in our logs I'd say it is an even mix between those clients that send content type and those that do not.

[christopherraa]

Sorry, my mistake here. Incomplete reading of the change you've made. It does indeed not break existing clients that don't send Content-Type or Accept. It does however break endpoints that for one reason or another don't have consumes and produces set.

{
  "errors": [
    {
      "message": "Expected  - got text/plain.",
      "path": "/header/Accept"
    }
  ],
  "status": 400
}

[jhthorsen]

I haven’t looked up the v2 spec in a while, but I’m pretty sure using consumes and produces is optional. I have however made a change to use this information after 4.xx IIRC.

Please ask on IRC or open an issue if you found a bug, as this issue if for v3. (Sorry, but I’m unsure what you try to point out in the last comment)

[divinity76]

related downstream issue, https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29595#