Refuse every name in a request that its operation does not declare

The router refused an undeclared query key only for an operation that
declared a query parameter or a request body.  An operation that declared
neither took any key, and a body sent to an operation with no requestBody
was never checked at all.  extract_query() merges the query, the fields of
the body and its files into the one hash that the callback gets.  So any
name in a request could stand in for a key that the callback meant to set
itself, and the callback cannot tell the two apart.

Now each operation is strict.  A query key must be a query parameter, or a
key that tPSGI sets itself.  A body to an operation with no requestBody is
a 400.  A field or a file must be a property of the schema of the media type
that was sent, unless that schema says additionalProperties, in which case
JSON::Validator decides as before.  The files reach validate() by name,
because extract_query() merged them into the query unchecked.

CGI mode stays as it was.  route() builds no query for an operation with
x-tpsgi-env, so nothing is validated, and the CGI reads the request from
%ENV.  It is how a CGI that reads whatever it is sent can move to tPSGI one
route at a time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Changes

@@ -7,3 +7,7 @@ Revision history for TPSGI
       Templates take {name} and a final {name+}, the most concrete template
       wins, and parameters are validated and coerced by JSON::Validator.
       A path with a method that it has no route for is a 405, not a 400.
+    - Every name in a request must be declared, or the request is a 400:
+      each key of the query, and each field and file of the body, unless a
+      body schema says additionalProperties.  An operation in CGI mode is
+      not validated.

README.md

@@ -471,11 +471,20 @@ operation in `$query->{dispatcher}`, and the template that matched in
 Before the callback runs, tPSGI checks the request against the operation's
 `parameters` and `requestBody`.  Values are coerced to their schema, so an
 integer arrives as a number.  A request that fails gets a 400, and the reasons
-go to the log at the debug level, not into the page.  The 400 is also the
-answer to a query key that the operation does not declare, when the operation
-declares a query parameter or a request body.  A form body is as strict as the
-`additionalProperties` of its schema.  A body whose media type is not among
-the `content` of the `requestBody` gets a 415.
+go to the log at the debug level, not into the page.  A body whose media type
+is not among the `content` of the `requestBody` gets a 415.
+
+Every name in the request must be declared, because each one reaches the
+callback as a key of its query, where it can stand in for a key that the
+callback meant to set itself.  So the 400 is also the answer to a query key
+that is not a query parameter, to a body sent to an operation that has no
+`requestBody`, and to a field or a file that is not a property of the body's
+schema.  A schema that says `additionalProperties`
+decides for itself which other properties it takes.
+
+CGI mode is the exception, for a CGI that reads whatever it is sent.  An
+operation with `x-tpsgi-env` is not validated, and its callback finds the
+request in %ENV.
 
 ## Callbacks
 

lib/TPSGI.pm

@@ -1049,6 +1049,7 @@ sub extract_query {
         $route,
         query        => $query,
         body         => $form,
+        uploads      => [ keys( %{ $uploads // {} } ) ],
         content_type => $env->{CONTENT_TYPE},
         env          => $env,
     );
@@ -1810,11 +1811,20 @@ C<< $query->{route_template} >>.
 Before the callback runs, tPSGI checks the request against the operation's
 C<parameters> and C<requestBody>.  Values are coerced to their schema, so an
 integer arrives as a number.  A request that fails gets a 400, and the reasons
-go to the log at the debug level, not into the page.  The 400 is also the
-answer to a query key that the operation does not declare, when the operation
-declares a query parameter or a request body.  A form body is as strict as the
-C<additionalProperties> of its schema.  A body whose media type is not among
-the C<content> of the C<requestBody> gets a 415.
+go to the log at the debug level, not into the page.  A body whose media type
+is not among the C<content> of the C<requestBody> gets a 415.
+
+Every name in the request must be declared, because each one reaches the
+callback as a key of its query, where it can stand in for a key that the
+callback meant to set itself.  So the 400 is also the answer to a query key
+that is not a query parameter, to a body sent to an operation that has no
+C<requestBody>, and to a field or a file that is not a property of the body's
+schema.  A schema that says C<additionalProperties>
+decides for itself which other properties it takes.
+
+CGI mode is the exception, for a CGI that reads whatever it is sent.  An
+operation with C<x-tpsgi-env> is not validated, and its callback finds the
+request in %ENV.
 
 =head2 Callbacks
 

lib/TPSGI/Router.pm

@@ -307,8 +307,9 @@ sub resolve_callback {
 =head2 rules_for($schema, $method, $template) = HASHREF
 
 What validate() checks of one operation beyond its schema: the names of its
-query parameters, whether an undeclared key in the query is a 400, and the
-media types of its request body.
+query parameters, the media types of its request body, and for each media type
+the names of the properties that its schema takes.  A schema that says
+C<additionalProperties> decides for itself, so it has no names here.
 
 =cut
 
@@ -318,11 +319,18 @@ sub rules_for {
     my @parameters = @{ $schema->parameters_for_request( [ $method, $template ] ) };
     my %declared   = map  { $_->{name} => 1 } grep { $_->{in} eq 'query' } @parameters;
     my ($body)     = grep { $_->{in} eq 'body' } @parameters;
+    my @types      = $body ? sort keys( %{ $body->{content} // {} } ) : ();
+
+    # get() rather than the content itself, because get() follows a $ref.
+    my %body_names = map {
+        my $of = $schema->get( [ 'paths', $template, $method, 'requestBody', 'content', $_, 'schema' ] ) // {};
+        exists $of->{additionalProperties} ? () : ( $_ => { map { $_ => 1 } keys( %{ $of->{properties} // {} } ) } );
+    } @types;
 
     return {
         query_names => \%declared,
-        strict      => ( %declared || $body ) ? 1                                            : 0,
-        body_types  => $body                  ? [ sort keys( %{ $body->{content} // {} } ) ] : undef,
+        body_types  => $body ? \@types : undef,
+        body_names  => \%body_names,
     };
 }
 
@@ -371,15 +379,30 @@ sub match {
 =head2 validate($route, %request) = LIST
 
 Check a request against the operation of the $route that match() returned.
-%request holds C<query> and C<body>, each a HASH reference, C<content_type>,
-and C<env>, the PSGI environment, from which the headers and cookies come.
+%request holds C<query> and C<body>, each a HASH reference, C<uploads>, the
+names of the files in the body, C<content_type>, and C<env>, the PSGI
+environment, from which the headers and cookies come.
 
 It returns an empty list when the request is valid.  Otherwise it returns a
 status and the reasons.  The status is 415 when the operation has a request
 body and the media type of the request is not among its C<content>.  It is 400
-when a parameter or the body does not match its schema, and when the query
-holds a key that the operation does not declare.  The last rule holds only for
-an operation that declares a query parameter or a request body.
+when a parameter or the body does not match its schema, and when the request
+holds a name that the operation does not declare:
+
+=over
+
+=item a key of the query that is not a query parameter, unless tPSGI sets
+that key itself;
+
+=item any field or file, when the operation has no C<requestBody>;
+
+=item a field or file that is not a property of the schema for its media type,
+unless that schema says C<additionalProperties>.
+
+=back
+
+An operation in CGI mode is never validated, because route() builds no query
+for it.
 
 The values of C<path>, C<query> and C<body> are coerced in place, so a parameter
 of type integer arrives as a number.
@@ -392,17 +415,22 @@ sub validate {
     my $query = $request{query} // {};
     my $body  = $request{body};
     my $env   = $request{env} // {};
+    my @sent  = sort( keys( %{ $body // {} } ), @{ $request{uploads} // [] } );
 
-    if ( $route->{body_types} && defined $body ) {
-        my $type = $request{content_type} // '';
-        return ( 415, "/body: '$type' is not one of @{ $route->{body_types} }" ) unless JSON::Validator::Util::negotiate_content_type( $route->{body_types}, $type );
-    }
+    if ( $route->{body_types} && ( defined $body || @sent ) ) {
+        my $type   = $request{content_type} // '';
+        my $actual = JSON::Validator::Util::negotiate_content_type( $route->{body_types}, $type );
+        return ( 415, "/body: '$type' is not one of @{ $route->{body_types} }" ) unless $actual;
 
-    if ( $route->{strict} ) {
-        my %known      = ( %{ $route->{query_names} }, map { $_ => 1 } @reserved );
-        my @undeclared = grep { !$known{$_} } sort keys(%$query);
-        return ( 400, map { "/$_: the operation does not declare this parameter" } @undeclared ) if @undeclared;
+        my $names = $route->{body_names}{$actual};
+        my @extra = $names ? grep { !$names->{$_} } @sent : ();
+        return ( 400, map { "/body/$_: the schema of $actual does not declare this property" } @extra ) if @extra;
     }
+    return ( 400, map { "/body/$_: the operation takes no body" } @sent ) if @sent && !$route->{body_types};
+
+    my %known      = ( %{ $route->{query_names} }, map { $_ => 1 } @reserved );
+    my @undeclared = grep { !$known{$_} } sort keys(%$query);
+    return ( 400, map { "/$_: the operation does not declare this parameter" } @undeclared ) if @undeclared;
 
     my %cookies = map { split( m/=/, $_, 2 ) } split( m/ ; [ ]* /, $env->{HTTP_COOKIE} // '' );
     my %headers = map {

t/TPSGI-Router.t

@@ -30,7 +30,12 @@ sub operation {
 
 sub document {
     my (%paths) = @_;
-    return { openapi => '3.0.3', info => { title => 'test', version => '1' }, paths => \%paths };
+    return {
+        openapi    => '3.0.3',
+        info       => { title   => 'test', version => '1' },
+        components => { schemas => { Count => { type => 'object', properties => { count => { type => 'integer' } } } } },
+        paths      => \%paths,
+    };
 }
 
 my $router = TPSGI::Router->new(
@@ -58,6 +63,32 @@ my $router = TPSGI::Router->new(
                 },
             ),
         },
+        '/upload' => {
+            post => operation(
+                requestBody => {
+                    content => {
+                        'multipart/form-data' => {
+                            schema => {
+                                type       => 'object',
+                                properties => { count => { type => 'integer' }, file => { type => 'string', format => 'binary' } },
+                            },
+                        },
+                        'application/x-www-form-urlencoded' => { schema => { '$ref' => '#/components/schemas/Count' } },
+                    },
+                },
+            ),
+        },
+        '/open' => {
+            post => operation(
+                requestBody => {
+                    content => {
+                        'application/x-www-form-urlencoded' => {
+                            schema => { type => 'object', additionalProperties => true, properties => { count => { type => 'integer' } } },
+                        },
+                    },
+                },
+            ),
+        },
         '/search' => {
             get => operation(
                 parameters => [
@@ -180,8 +211,11 @@ subtest 'validation' => sub {
     ( $status, @errors ) = $router->validate( $route, query => { limit => 1, junk => 1 } );
     is( $status, 400, 'an undeclared query key is a 400' );
     like( \@errors, [qr/\/junk:/], 'and named' );
-    is( [ $router->validate( $route,                            query => { limit    => 1, route => '/x' } ) ], [], 'except a key that tPSGI sets itself' );
-    is( [ $router->validate( $router->match( 'GET', '/loose' ), query => { anything => 1 } ) ],                [], 'and an operation that declares nothing takes anything' );
+    is( [ $router->validate( $route, query => { limit => 1, route => '/x' } ) ], [], 'except a key that tPSGI sets itself' );
+    ( $status, @errors ) = $router->validate( $router->match( 'GET', '/loose' ), query => { anything => 1 } );
+    is( $status, 400, 'and so is any key of the query, for an operation that declares nothing' );
+    like( \@errors, [qr/\/anything:/], 'named too' );
+    is( [ $router->validate( $router->match( 'GET', '/loose' ), query => {} ) ], [], 'which takes an empty query' );
 
     $route = $router->match( 'POST', '/form' );
     my $form_type = 'application/x-www-form-urlencoded';
@@ -194,6 +228,35 @@ subtest 'validation' => sub {
     is( [ $router->validate( $route, body => { count => 3 }, content_type => "$form_type; charset=UTF-8" ) ], [],  'and parameters of the media type do not count' );
 };
 
+subtest 'the names of a body' => sub {
+    my $form_type = 'application/x-www-form-urlencoded';
+    my $multipart = 'multipart/form-data; boundary=x';
+
+    my ( $status, @errors ) = $router->validate( $router->match( 'GET', '/loose' ), body => { count => 3 }, content_type => $form_type );
+    is( $status, 400, 'a field, to an operation with no requestBody, is a 400' );
+    like( \@errors, [qr/\/body\/count:[ ]the[ ]operation[ ]takes[ ]no[ ]body/], 'for that reason' );
+    ( $status, @errors ) = $router->validate( $router->match( 'GET', '/loose' ), body => {}, uploads => ['file'], content_type => $multipart );
+    is( $status, 400, 'and so is a file' );
+    like( \@errors, [qr/\/body\/file:/], 'named' );
+    is( [ $router->validate( $router->match( 'GET', '/loose' ), body => {}, content_type => $form_type ) ], [], 'but an empty body is not' );
+
+    my $route = $router->match( 'POST', '/upload' );
+    is( [ $router->validate( $route, body => { count => 3 }, uploads => ['file'], content_type => $multipart ) ], [], 'a field and a file that the schema names pass' );
+    ( $status, @errors ) = $router->validate( $route, body => { count => 3, junk => 1 }, content_type => $multipart );
+    is( $status, 400, 'a field that a schema without additionalProperties does not name is a 400' );
+    like( \@errors, [qr/\/body\/junk:[ ]the[ ]schema[ ]of[ ]multipart\/form-data/], 'which names the media type' );
+    ( $status, @errors ) = $router->validate( $route, body => { count => 3 }, uploads => ['other'], content_type => $multipart );
+    is( $status, 400, 'and so is a file that it does not name' );
+    like( \@errors, [qr/\/body\/other:/], 'named' );
+
+    ( $status, @errors ) = $router->validate( $route, body => { count => 3, file => 'x' }, content_type => $form_type );
+    is( $status, 400, 'the names are those of the media type that was sent, here a $ref' );
+    like( \@errors, [qr/\/body\/file:[ ]the[ ]schema[ ]of[ ]\Q$form_type\E/], 'which does not name the file' );
+    is( [ $router->validate( $route, body => { count => 3 }, content_type => $form_type ) ], [], 'and does name the count' );
+
+    is( [ $router->validate( $router->match( 'POST', '/open' ), body => { count => 3, junk => 1 }, content_type => $form_type ) ], [], 'a schema with additionalProperties takes what it says it takes' );
+};
+
 subtest 'precedence' => sub {
     my @order = map { $_->{template} } grep { index( $_->{template}, '/sitemap/' ) == 0 || index( $_->{template}, '/lib/' ) == 0 } $router->routes();
     is( \@order, [qw{/lib/index /sitemap/static.xml /sitemap/{map}.xml.gz /sitemap/{map}.xml /lib/{module+}}], 'literal routes first, then by segment, then by literal length' );

t/TPSGI.t

@@ -179,6 +179,19 @@ subtest 'new' => sub {
                         responses => { 200 => { description => 'ok' } },
                     },
                 },
+                '/cgi' => {
+                    get => {
+                        'x-tpsgi-callback' => sub { return [ 200, [], [ $ENV{QUERY_STRING} ] ] },
+                        'x-tpsgi-env'      => true,
+                        responses          => { 200 => { description => 'ok' } },
+                    },
+                },
+                '/plain' => {
+                    post => {
+                        'x-tpsgi-callback' => sub { return [ 200, [], ['ok'] ] },
+                        responses          => { 200 => { description => 'ok' } },
+                    },
+                },
             },
         );
         ROUTER
@@ -213,6 +226,29 @@ subtest 'extract_query' => sub {
     is( $res->[0], 400, 'even when the path declares it' );
 };
 
+subtest 'extract_query, with a body' => sub {
+    my $tpsgi = TPSGI->new( %new_options, routers => ['GoodRouter.pm'] );
+    my $part  = qq{--x\r\nContent-Disposition: form-data; name="file"; filename="f.txt"\r\nContent-Type: text/plain\r\n\r\nhi\r\n--x--\r\n};
+    my %env   = (
+        REQUEST_METHOD => 'POST',
+        CONTENT_TYPE   => 'multipart/form-data; boundary=x',
+        CONTENT_LENGTH => length($part),
+    );
+
+    local @Test::Logger::Capture::lines;
+    open( my $input, '<', \$part ) or die "Cannot open a scalar: $!";
+    my $res = $tpsgi->extract_query( '/plain', $tpsgi->routes->match( 'POST', '/plain' ), { %env, 'psgi.input' => $input } );
+    close($input) or die "Cannot close: $!";
+    is( ref $res eq 'ARRAY' && $res->[0], 400, 'a file, to an operation with no requestBody, is a 400' );
+};
+
+subtest 'route, in CGI mode' => sub {
+    my $tpsgi = TPSGI->new( %new_options, routers => ['GoodRouter.pm'] );
+    my $res   = $tpsgi->respond( { REQUEST_METHOD => 'GET', PATH_INFO => '/cgi', REQUEST_URI => '/cgi?anything=1', QUERY_STRING => 'anything=1', HTTP_HOST => 'test.test' } );
+    is( $res->[0],    200,          'is not validated, so a key that nothing declares is no 400' );
+    is( $res->[2][0], 'anything=1', 'and the CGI finds it in %ENV' );
+};
+
 subtest 'respond, with a method that the path does not have' => sub {
     my $tpsgi = TPSGI->new( %new_options, routers => ['GoodRouter.pm'] );
     my $res   = $tpsgi->respond( { REQUEST_METHOD => 'POST', PATH_INFO => '/post/x', REQUEST_URI => '/post/x', HTTP_HOST => 'test.test' } );