From 006964710049b8ba3d547d8722fcf0fed034adb6 Mon Sep 17 00:00:00 2001 From: George Clark Date: Sat, 29 Oct 2016 23:46:45 -0400 Subject: [PATCH] Item13897: Fix more JSON Rpc tests and issues: SMELL: Two failing tests: * F: JsonrpcTests::test_post_required The _trigger_method routine is not causing a JSON error for "get" * F: JsonrpcTests::test_simple_query_params The "defaultweb" is not being applied. --- JsonRpcContrib/lib/Foswiki/Request/JSON.pm | 24 +- .../test/unit/JsonRpcContrib/JsonrpcTests.pm | 237 +++++++++++++----- core/lib/Foswiki/Request/Rest.pm | 2 +- 3 files changed, 194 insertions(+), 69 deletions(-) diff --git a/JsonRpcContrib/lib/Foswiki/Request/JSON.pm b/JsonRpcContrib/lib/Foswiki/Request/JSON.pm index 0cdb1ccd68..667adbc8f3 100644 --- a/JsonRpcContrib/lib/Foswiki/Request/JSON.pm +++ b/JsonRpcContrib/lib/Foswiki/Request/JSON.pm @@ -167,10 +167,14 @@ sub parseJSON { $data = ( ref($foo) eq 'ARRAY' ) ? shift @$foo : $foo; + my $minimal = ($data) ? 0 : 1; $data ||= '{"jsonrpc":"2.0"}'; # Minimal setup $jsondata = $this->initFromString($data); + # If the parse failed, give up here. + return $jsondata if $this->jsonerror; + # some basic checks if this is a proper json-rpc 2.0 request # must have a version tag @@ -183,13 +187,16 @@ sub parseJSON { ); } - # must have a json method + # must have a json method + # SMELL: This error is suppressed for simple query param type json requests. + # It's a processing order issue. The method is there in the query path, but it + # has not been parsed yet. $this->jsonerror( new Foswiki::Contrib::JsonRpcContrib::Error( code => -32600, text => "Invalid JSON-RPC request - no method" ) - ) unless defined $jsondata->{method}; + ) unless $minimal || defined $jsondata->{method}; # must not have any other keys other than these foreach my $key ( keys %{$jsondata} ) { @@ -239,7 +246,7 @@ sub initFromString { =begin TML ----++ private objectMethod _establishAddress() -> n/a +---++ private objectMethod _establishAttributes() -> n/a Used internally by the web() and topic() methods to trigger parsing of the JSON topic parameter or the CGI topic parameer, and set object variables with the results. @@ -262,6 +269,15 @@ around _establishAttributes => sub { $parse->{topic} = ucfirst( $parse->{topic} ) if ( defined $parse->{topic} ); + # SMELL. This isn't working. Test still fails with wrong web + unless ( defined $parse->{web} ) { + if ( defined $this->param('defaultweb') ) { + $parse->{web} = + Foswiki::Sandbox::untaint( $this->param('defaultweb'), + \&Foswiki::Sandbox::validateWebName ); + } + } + # Note that Web can still be undefined. Caller then determines if the # defaultweb query param, or the HomeWeb config parameter should be used. @@ -433,7 +449,7 @@ sub _establishNamespace { $this->jsonerror( new Foswiki::Contrib::JsonRpcContrib::Error( code => -32600, - text => "Invalid Namespace / method" + text => "Invalid Namespace / method FOO" ) ); return; diff --git a/JsonRpcContrib/test/unit/JsonRpcContrib/JsonrpcTests.pm b/JsonRpcContrib/test/unit/JsonRpcContrib/JsonrpcTests.pm index 6d6dd25cef..ce3ed0c400 100644 --- a/JsonRpcContrib/test/unit/JsonRpcContrib/JsonrpcTests.pm +++ b/JsonRpcContrib/test/unit/JsonRpcContrib/JsonrpcTests.pm @@ -35,10 +35,11 @@ sub json_handler { die "incorrect jsonmethod()" unless $request->jsonmethod() eq 'trial'; # The old method() was jsonmethod alias. - #die "incorrect method()" unless $request->method() eq 'trial'; - die "incorrect method()" unless $request->method eq 'post'; - die "Incorrect topic" unless $app->request->topic eq 'WebChanges'; - die "Incorrect web" unless $app->request->web eq 'System'; + die "incorrect method()" unless $request->jsonmethod() eq 'trial'; + die "incorrect method()" unless $request->method eq 'post'; + die "Incorrect topic" unless $app->request->topic eq 'WebChanges'; + die "Incorrect web " . $app->request->web + unless $app->request->web eq 'System'; return 'SUCCESS'; } @@ -89,16 +90,8 @@ sub test_simple_postdata { sub { $this->createNewFoswikiApp( - requestParams => { - initializer => { - action => ['jsonrpc'], - uri => '/' . __PACKAGE__ . '/trial', - path_info => '/' . __PACKAGE__ . '/trial', - }, - }, engineParams => { initialAttributes => { - uri => '/' . __PACKAGE__ . "/trial", path_info => '/' . __PACKAGE__ . "/trial", method => 'post', action => 'jsonrpc', @@ -127,87 +120,203 @@ sub test_simple_postdata { $this->assert_matches( qr/"result" : "SUCCESS"/, $response ); return; } -1; -__END__ -# Simple jsonrpc, using query params -sub test_simple_query_params { +# -32600: Invalid Request - The JSON sent is not a valid Request object. +sub test_invalid_request { my $this = shift; - Foswiki::Contrib::JsonRpcContrib::registerMethod( __PACKAGE__, 'trial', - \&json_handler ); - my $query = Unit::Request::JSON->new( { action => ['jsonrpc'], } ); - $query->path_info( '/' . __PACKAGE__ . '/trial' ); - $query->method('post'); - $query->param( 'topic', 'WebChanges' ); - $query->param( 'defaultweb', 'System' ); - $this->createNewFoswikiSession( $this->{test_user_login}, $query ); - my ( $response, $result, $out, $err ) = - $this->capture( $UI_FN, $this->{session} ); + #$query->path_info( '/' . __PACKAGE__ . '/saywhat' ); + # + my $response; + + # This first test has a syntax error - missing { + try { + ($response) = $this->capture( + sub { + + $this->createNewFoswikiApp( + engineParams => { + initialAttributes => { + path_info => '/' . __PACKAGE__ . "/trial", + method => 'post', + action => 'jsonrpc', + postData => +'{"jsonrpc":"1.0","method":"trial","params":"wizard":"ScriptHash","method":"verify","keys":"{ScriptUrlPaths}{view}","set":{},"topic":"System.WebChanges","cfgpassword":"xxxxxxx"},"id":"iCall-verify_6"}' + }, + }, + ); + Foswiki::Contrib::JsonRpcContrib::registerMethod( __PACKAGE__, + 'trial', \&json_handler ); + + return $this->app->handleRequest; + }, + ); + } + catch { + my $e = $_; + if ( ref($e) && $e->isa('Foswiki::EngineException') ) { + $this->assert_equals( 401, $e->status, $e->stringify ); + } + else { + $e->rethrow; + } + }; + + $this->assert_matches( qr/"code" : -32700/, $response ); + $this->assert_matches( + qr/"message" : "Parse error - invalid json-rpc request:/, $response ); + + # Now test with an invalid version + try { + ($response) = $this->capture( + sub { + + $this->createNewFoswikiApp( + engineParams => { + initialAttributes => { + path_info => '/' . __PACKAGE__ . "/trial", + method => 'post', + action => 'jsonrpc', + postData => +'{"jsonrpc":"1.0","method":"trial","params":{"wizard":"ScriptHash","method":"verify","keys":"{ScriptUrlPaths}{view}","set":{},"topic":"System.WebChanges","cfgpassword":"xxxxxxx"},"id":"iCall-verify_6"}', + }, + }, + ); + Foswiki::Contrib::JsonRpcContrib::registerMethod( __PACKAGE__, + 'trial', \&json_handler ); + + return $this->app->handleRequest; + }, + ); + } + catch { + my $e = $_; + if ( ref($e) && $e->isa('Foswiki::EngineException') ) { + $this->assert_equals( 401, $e->status, $e->stringify ); + } + else { + $e->rethrow; + } + }; + + $this->assert_matches( qr/"code" : -32600/, $response ); + $this->assert_matches( qr/"message" : "Invalid JSON-RPC request/, + $response ); - $this->assert_matches( qr/"result" : "SUCCESS"/, $response ); return; } -# -32601: Method not found - The method does not exist / is not available. -sub test_method_missing { +# Simple jsonrpc, using query params +sub test_simple_query_params { my $this = shift; - Foswiki::Contrib::JsonRpcContrib::registerMethod( __PACKAGE__, 'trial', - \&json_handler ); - my $query = Unit::Request::JSON->new( { action => ['jsonrpc'], } ); - $query->path_info( '/' . __PACKAGE__ . '/saywhat' ); - $query->method('post'); - $query->param( 'topic', 'WebChanges' ); - $query->param( 'defaultweb', 'System' ); - $this->createNewFoswikiSession( $this->{test_user_login}, $query ); - my ( $response, $result, $out, $err ) = - $this->capture( $UI_FN, $this->{session} ); + my $response; - $this->assert_matches( qr/"code" : -32601/, $response ); + try { + ($response) = $this->capture( + sub { + + $this->createNewFoswikiApp( + requestParams => { + initializer => { + action => ['jsonrpc'], + uri => '/' . __PACKAGE__ . '/trial', + path_info => '/' . __PACKAGE__ . '/trial', + topic => 'WebChanges', + defaultweb => 'System', + }, + }, + engineParams => { + initialAttributes => { + uri => '/' . __PACKAGE__ . "/trial", + path_info => '/' . __PACKAGE__ . "/trial", + method => 'post', + action => 'jsonrpc', + }, + }, + ); + Foswiki::Contrib::JsonRpcContrib::registerMethod( __PACKAGE__, + 'trial', \&json_handler ); + + return $this->app->handleRequest; + }, + ); + } + catch { + my $e = $_; + if ( ref($e) && $e->isa('Foswiki::EngineException') ) { + $this->assert_equals( 401, $e->status, $e->stringify ); + } + else { + $e->rethrow; + } + }; + $this->assert_matches( qr/"result" : "SUCCESS"/, $response ); return; } -# -32600: Invalid Request - The JSON sent is not a valid Request object. -sub test_invalid_request { +sub test_post_required { my $this = shift; - Foswiki::Contrib::JsonRpcContrib::registerMethod( __PACKAGE__, 'trial', - \&json_handler ); - my $query = Unit::Request::JSON->new( { action => ['jsonrpc'], } ); - $query->path_info( '/' . __PACKAGE__ . '/saywhat' ); - $query->param( 'POSTDATA', -'{"jsonrpc":"2.0","method":"trial","params":"wizard":"ScriptHash","method":"verify","keys":"{ScriptUrlPaths}{view}","set":{},"topic":"System.WebChanges","cfgpassword":"xxxxxxx"},"id":"iCall-verify_6"}' - ); - $this->createNewFoswikiSession( $this->{test_user_login}, $query ); - my ( $response, $result, $out, $err ) = - $this->capture( $UI_FN, $this->{session} ); - $this->assert_matches( qr/"code" : -32600/, $response ); - $this->assert_matches( - qr/"message" : "Invalid JSON-RPC request - must be jsonrpc: '2.0'"/, - $response ); + my $response; + + try { + ($response) = $this->capture( + sub { + + $this->createNewFoswikiApp( + engineParams => { + initialAttributes => { + path_info => '/' . __PACKAGE__ . "/trial", + method => 'get', + action => 'jsonrpc', + postData => +'{"jsonrpc":"2.0","method":"trial","params":{"wizard":"ScriptHash","method":"verify","keys":"{ScriptUrlPaths}{view}","set":{},"topic":"System.WebChanges","cfgpassword":"xxxxxxx"},"id":"iCall-verify_6"}', + }, + }, + ); + Foswiki::Contrib::JsonRpcContrib::registerMethod( __PACKAGE__, + 'trial', \&json_handler ); + + return $this->app->handleRequest; + }, + ); + } + catch { + my $e = $_; + if ( ref($e) && $e->isa('Foswiki::EngineException') ) { + $this->assert_equals( 401, $e->status, $e->stringify ); + } + else { + $e->rethrow; + } + }; + + $this->assert_matches( qr/"code" : -32600/, $response ); + $this->assert_matches( qr/"message" : "Method must be POST"/, $response ); return; } -sub test_post_required { +1; +__END__ +# -32601: Method not found - The method does not exist / is not available. +sub test_method_missing { my $this = shift; Foswiki::Contrib::JsonRpcContrib::registerMethod( __PACKAGE__, 'trial', \&json_handler ); my $query = Unit::Request::JSON->new( { action => ['jsonrpc'], } ); - $query->method('get'); - $query->path_info( '/' . __PACKAGE__ . '/trial' ); - $query->param( 'POSTDATA', -'{"jsonrpc":"2.0","method":"trial","params":{"wizard":"ScriptHash","method":"verify","keys":"{ScriptUrlPaths}{view}","set":{},"topic":"WebChanges","cfgpassword":"xxxxxxx"},"id":"iCall-verify_6"}' - ); + $query->path_info( '/' . __PACKAGE__ . '/saywhat' ); + $query->method('post'); + $query->param( 'topic', 'WebChanges' ); + $query->param( 'defaultweb', 'System' ); $this->createNewFoswikiSession( $this->{test_user_login}, $query ); my ( $response, $result, $out, $err ) = $this->capture( $UI_FN, $this->{session} ); - $this->assert_matches( qr/"code" : -32600/, $response ); - $this->assert_matches( qr/"message" : "Method must be POST"/, $response ); + $this->assert_matches( qr/"code" : -32601/, $response ); return; } diff --git a/core/lib/Foswiki/Request/Rest.pm b/core/lib/Foswiki/Request/Rest.pm index 4d5a4fb8d4..cc451fac28 100644 --- a/core/lib/Foswiki/Request/Rest.pm +++ b/core/lib/Foswiki/Request/Rest.pm @@ -49,7 +49,7 @@ has invalidVerb => ( =begin TML ----++ private objectMethod _establishAddress() -> n/a +---++ private objectMethod _establishAttributes() -> n/a Used internally by the web(), topic() and attachment() methods to trigger parsing of the url and/or topic= parameter and set object variables with the results. Attachment requests have to also accommodate redirect requests