Merge pull request #8 from oetiker/content-type

check content-type of incoming requests
oetiker · Nov 12, 2020 · 7a8fc8b · 7a8fc8b
2 parents ba5957f + 2e74e1c
commit 7a8fc8b
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 35 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,7 @@
 Revision history for Mojolicious::Pugin::Qooxdoo
 
+1.0.9 2020-11-12
+  - only accept post requests with proper content type
 1.0.7 2019-11-12
   - use log method on controller to show request context in log
 1.0.5 2019-03-09

diff --git a/lib/Mojolicious/Plugin/Qooxdoo.pm b/lib/Mojolicious/Plugin/Qooxdoo.pm
@@ -4,7 +4,7 @@ use Mojo::Base 'Mojolicious::Plugin';
 use File::Spec::Functions qw(splitdir updir catdir file_name_is_absolute);
 use Cwd qw(abs_path);
 
-our $VERSION = '1.0.8';
+our $VERSION = '1.0.9';
 
 sub register {
     my ($self, $app, $conf) = @_;

diff --git a/lib/Mojolicious/Plugin/Qooxdoo/JsonRpcController.pm b/lib/Mojolicious/Plugin/Qooxdoo/JsonRpcController.pm
@@ -13,7 +13,7 @@ use Encode;
 
 has toUTF8 => sub { find_encoding('utf8') };
 
-our $VERSION = '1.0.7';
+our $VERSION = '1.0.9';
 
 has 'service';
 
@@ -43,27 +43,35 @@ sub dispatch {
     for ( $self->req->method ){
         /^POST$/ && do {
             # Data comes as JSON object, so fetch a reference to it
+            my $type = $self->req->headers->content_type//'*missing header*';
+            if ($type !~ m{^application/json\b}i) {
+                $log->error("unexpected Content-Type header: $type (should be application/json)");
+                $self->render(text => "invalid payload format announcement", status=>500);
+                return;
+            }
             $data = eval { decode_json($self->req->body) };
-	    	if ($@) {
-				my $error = "Invalid json string: " . $@;
-				$log->error($error);
-				$self->render(text => $error, status=>500);
-				return;
-	    	};
+            if ($@) {
+                my $error = "Invalid json string: " . $@;
+                $log->error($error);
+                $self->render(text => "invalid payload format", status=>500);
+                return;
+            };
             $self->requestId($data->{id});
             $self->crossDomain(0);
             last;
         };
         /^GET$/ && do {
-
+            # not checking the content header here since we are trying to
+            # to a cross domain request ... all sorts of things may have
+            # happened to the data since this
             $data= eval { decode_json($self->param('_ScriptTransport_data')) };
 
             if ($@) {
-				my $error = "Invalid json string: " . $@;
-				$log->error($error);
-				$self->render(text => $error, status=>500);
-				return;
-	    	};
+                my $error = "Invalid json string: " . $@;
+                $log->error($error);
+                $self->render(text => $error, status=>500);
+                return;
+            };
 
             $self->requestId($self->param('_ScriptTransport_id')) ;
             $self->crossDomain(1);
@@ -182,8 +190,8 @@ sub logRpcCall {
 }
 
 sub renderJsonRpcResult {
-	my $self = shift;
-	my $data = shift;
+    my $self = shift;
+    my $data = shift;
     my $reply = { id => $self->requestId, result => $data };
     $self->logRpcReturn(dclone($reply));
     $self->finalizeJsonRpcReply(encode_json($reply));
@@ -203,10 +211,10 @@ sub logRpcReturn {
 }
 
 sub renderJsonRpcError {
-	my $self = shift;
-	my $exception = shift;
-	my $error;
-	for (ref $exception){
+    my $self = shift;
+    my $exception = shift;
+    my $error;
+    for (ref $exception){
         /HASH/ && $exception->{message} && do {
             $error = {
                 origin => $exception->{origin} || 2, 
@@ -234,8 +242,8 @@ sub renderJsonRpcError {
 }
 
 sub finalizeJsonRpcReply {
-	my $self  = shift;
-	my $reply = shift;
+    my $self  = shift;
+    my $reply = shift;
     if ($self->crossDomain){
         # for GET requests, qooxdoo expects us to send a javascript method
         # and to wrap our json a litte bit more

diff --git a/t/simple.t b/t/simple.t
@@ -28,43 +28,47 @@ $t->put_ok('/root/jsonrpc','{"hello": dummy}')
   ->status_is(500);
 
 $t->post_ok('/root/jsonrpc','{"hello": dummy}')
-  ->content_like(qr/Invalid json string/i,'bad request identified')
+  ->content_like(qr/invalid payload format announcement/i,'bad request identified')
   ->status_is(500);
 
-$t->post_ok('/root/jsonrpc','{"ID":1,"method":"test"}')
+$t->post_ok('/root/jsonrpc',{ 'Content-Type' => 'application/json' }, '{"hello": dummy}')
+  ->content_like(qr/invalid payload format/i,'bad request identified')
+  ->status_is(500);
+
+$t->post_ok('/root/jsonrpc', json => {ID => 1,method=>"test"})
   ->content_like(qr/Missing 'id' property in JsonRPC request/,'bad request identified')
   ->status_is(500);
 
-$t->post_ok('/root/jsonrpc','{"id":1,"method":"test"}')
+$t->post_ok('/root/jsonrpc', json => {"id"=>1,"method" => "test"})
   ->content_like(qr/Missing service property/,'missing service found');
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"test"}')
+$t->post_ok('/root/jsonrpc', json => {"id" =>1,"service"=>"test"})
   ->content_like(qr/Missing method property/, 'missing method found');
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"test","method":"test"}')
+$t->post_ok('/root/jsonrpc', json => {"id" => 1,"service" => "test","method"=>"test"})
   ->json_is('',{error=>{origin=>1,code=>2,message=>"service test not available"},id=>1},'json error for invalid service')
   ->content_type_is('application/json; charset=utf-8')
   ->status_is(200);
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"rpc","method":"test"}')
+$t->post_ok('/root/jsonrpc', json => {"id"=>1,"service"=>"rpc","method"=>"test"})
   ->json_is('',{error=>{origin=>1,code=>6,message=>"rpc access to method test denied"},id=>1},'json error for invalid method');
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"rpc","method":"echo"}')
+$t->post_ok('/root/jsonrpc', json => {"id"=>1,"service" => "rpc","method" =>"echo"})
   ->json_is('',{error=>{origin=>2,code=>123,message=>"Argument Required!"},id=>1},'propagating generic exception');
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"rpc","method":"echo","params":["hello"]}')
+$t->post_ok('/root/jsonrpc', json => {"id" => 1,"service" => "rpc","method" => "echo","params" => ["hello"]})
   ->json_is('',{id=>1,result=>'hello'},'post request');
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"rpc","method":"async","params":["hello"]}')
+$t->post_ok('/root/jsonrpc', json => {"id"=>1,"service"=>"rpc","method"=>"async","params" => ["hello"]})
   ->json_is('',{id=>1,result=>'Delayed hello for 1.5 seconds!'},'async request');
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"rpc","method":"asyncException","params":[]}')
+$t->post_ok('/root/jsonrpc', json => {"id" => 1,"service"=>"rpc","method"=>"asyncException","params"=>[]})
   ->json_is('',{id=>1,error=>{origin=>2,code=>334,message=>'a simple error'}},'async exception');
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"rpc","method":"async_p","params":["hello"]}')
-  ->json_is('',{id=>1,result=>'Delayed hello for 1.5 seconds!'},'promise request');
 
-$t->post_ok('/root/jsonrpc','{"id":1,"service":"rpc","method":"asyncException_p","params":[]}')
+$t->post_ok('/root/jsonrpc',json => {"id" => 1,"service" => "rpc","method" => "async_p","params" => ["hello"]})
+  ->json_is('',{id=>1,result=>'Delayed hello for 1.5 seconds!'},'promise request');
+$t->post_ok('/root/jsonrpc', json => {"id"=>1,"service"=>"rpc","method"=>"asyncException_p","params"=>[]})
   ->json_is('',{id=>1,error=>{origin=>2,code=>334,message=>'a simple error'}},'promise exception');
 
 $t->get_ok('/root/jsonrpc?_ScriptTransport_id=1&_ScriptTransport_data="id":1,"service":"rpc","method":"echo","params":["hello"]}')