fu: Close-up of Fu, bringing a scoop of water to her mouth (Default)
fu ([personal profile] fu) wrote in [site community profile] changelog2011-02-28 10:47 am
  • Add Memory
  • Share This Entry

[dw-free] Make sure method is GET/POST ( unless the controller handles more )

[commit: http://hg.dwscoalition.org/dw-free/rev/b0d94d3528e4]

http://bugs.dwscoalition.org/show_bug.cgi?id=3539

Limit the methods that can be used for the atom publishing interface (update
to use cleaner method rather than hacking in at the end). Create an
$r->HTTP_OK method, as opposed to $r->OK, for accurate tests.

Patch by [personal profile] fu.

Files modified:
  • cgi-bin/DW/Controller/Interface/AtomAPI.pm
  • cgi-bin/DW/Request/Apache2.pm
  • cgi-bin/DW/Request/Standard.pm
  • t/atom-post.t
--------------------------------------------------------------------------------
diff -r 37f7a2c8953c -r b0d94d3528e4 cgi-bin/DW/Controller/Interface/AtomAPI.pm
--- a/cgi-bin/DW/Controller/Interface/AtomAPI.pm	Mon Feb 28 18:44:54 2011 +0800
+++ b/cgi-bin/DW/Controller/Interface/AtomAPI.pm	Mon Feb 28 18:47:21 2011 +0800
@@ -28,13 +28,17 @@ require 'ljprotocol.pl';
 require 'ljprotocol.pl';
 
 # service document URL is the same for all users
-DW::Routing->register_string( "/interface/atom",   \&service_document, app => 1, format => "atom" );
+DW::Routing->register_string( "/interface/atom",   \&service_document, app => 1,
+        format => "atom", methods => { GET => 1 } );
 
 # note: safe to put these pages in the user subdomain even if they modify data
 # because we don't use cookies (so even if a user's cookies are stolen...)
-DW::Routing->register_string( "/interface/atom/entries", \&entries_handler, user => 1, format => "atom" );
-DW::Routing->register_string( "/interface/atom/entries/tags", \&categories_document, user => 1, format => "atom" );
-DW::Routing->register_regex( qr#/interface/atom/entries/(\d+)#, \&entry_handler, user => 1, format => "atom" );
+DW::Routing->register_string( "/interface/atom/entries", \&entries_handler, user => 1,
+        format => "atom", methods => { POST => 1, GET => 1 } );
+DW::Routing->register_string( "/interface/atom/entries/tags", \&categories_document, user => 1,
+        format => "atom", methods => { GET => 1 } );
+DW::Routing->register_regex( qr#/interface/atom/entries/(\d+)#, \&entry_handler, user => 1,
+        format => "atom", methods => { GET => 1, PUT => 1, DELETE => 1 } );
 
 sub ok {
     my ( $message, $status, $content_type ) = @_;
@@ -190,10 +194,6 @@ sub service_document {
 
     my $r = DW::Request->get;
 
-    my $method = $r->method;
-    return err( "URI scheme /interface/atom/ is incompatible with $method" )
-        unless $method eq "GET";
-
     # FIXME: use XML::Atom::Service?
     my $ret = qq{<?xml version="1.0"?>};
     $ret .= qq{<service xmlns="http://www.w3.org/2007/app" xmlns:atom="http://www.w3.org/2005/Atom">};
@@ -222,8 +222,6 @@ sub categories_document {
 
     my $r = DW::Request->get;
 
-    return err( "URI scheme /interface/atom/entries/tags is incompatible with " . $r->method ) unless $r->method eq "GET";
-
     my $ret = qq{<?xml version="1.0"?>};
     $ret .= qq{<categories xmlns="http://www.w3.org/2007/app" xmlns:atom="http://www.w3.org/2005/Atom">};
 
@@ -251,8 +249,6 @@ sub entries_handler {
     my $r = DW::Request->get;
     return _create_entry( %$rv ) if $r->method eq "POST";
     return _list_entries( %$rv ) if $r->method eq "GET";
-
-    return err( "URI scheme /interface/atom/entries is incompatible with " . $r->method );
 }
 
 sub _create_entry {
@@ -400,8 +396,6 @@ sub entry_handler {
     return _retrieve_entry( %$rv, item => $olditem->{events}->[0], entry_obj => $entry_obj ) if $r->method eq "GET";
     return _edit_entry( %$rv, item => $olditem->{events}->[0], entry_obj => $entry_obj ) if $r->method eq "PUT";
     return _delete_entry( %$rv, item => $olditem->{events}->[0], entry_obj => $entry_obj ) if $r->method eq "DELETE";
-
-    return err( "URI scheme /interface/atom/entries/$jitemid is incompatible with " . $r->method );
 }
 
 sub _retrieve_entry {
diff -r 37f7a2c8953c -r b0d94d3528e4 cgi-bin/DW/Request/Apache2.pm
--- a/cgi-bin/DW/Request/Apache2.pm	Mon Feb 28 18:44:54 2011 +0800
+++ b/cgi-bin/DW/Request/Apache2.pm	Mon Feb 28 18:47:21 2011 +0800
@@ -274,6 +274,11 @@ sub OK {
     return Apache2::Const::OK;
 }
 
+sub HTTP_OK {
+    my DW::Request::Apache2 $self = $_[0];
+    return Apache2::Const::HTTP_OK;
+}
+
 sub HTTP_CREATED {
     return Apache2::Const::HTTP_CREATED;
 }
diff -r 37f7a2c8953c -r b0d94d3528e4 cgi-bin/DW/Request/Standard.pm
--- a/cgi-bin/DW/Request/Standard.pm	Mon Feb 28 18:44:54 2011 +0800
+++ b/cgi-bin/DW/Request/Standard.pm	Mon Feb 28 18:47:21 2011 +0800
@@ -288,7 +288,12 @@ sub call_bml {
 }
 
 # constants sometimes used
-sub OK        { return 200; }
+
+# indicates that this request has been handled
+sub OK        { return 0; }
+
+# HTTP status codes
+sub HTTP_OK { return 200; }
 sub HTTP_CREATED { return 201; }
 sub REDIRECT  { return 302; }
 sub NOT_FOUND { return 404; }
diff -r 37f7a2c8953c -r b0d94d3528e4 t/atom-post.t
--- a/t/atom-post.t	Mon Feb 28 18:44:54 2011 +0800
+++ b/t/atom-post.t	Mon Feb 28 18:47:21 2011 +0800
@@ -65,7 +65,8 @@ sub do_request {
     my %routing_data = ();
     $routing_data{username} = $user_subdomain if $user_subdomain;
 
-    DW::Routing->call( %routing_data );
+    my $returned = DW::Routing->call( %routing_data );
+    $r->status( $returned ) unless $returned eq $r->OK;
 }
 
 # check subject, etc
@@ -121,19 +122,18 @@ is( $r->status, $r->HTTP_UNAUTHORIZED, "
 is( $r->status, $r->HTTP_UNAUTHORIZED, "Passed wrong authorization information." );
 
 do_request( GET => $u->atom_service_document, authenticate => 1 );
-is( $r->status, $r->OK, "Successful authentication." );
+is( $r->status, $r->HTTP_OK, "Successful authentication." );
 
 
 note( "Service document introspection." );
 do_request( POST => $u->atom_service_document ); 
-is( $r->status, $r->HTTP_UNAUTHORIZED, "Service document protected by authorization." );
+is( $r->status, $r->HTTP_METHOD_NOT_ALLOWED, "Service document needs GET (even unauthorized)" );
 
 do_request( POST => $u->atom_service_document, authenticate => 1 );
-is( $r->status, $r->NOT_FOUND, "Service document needs GET." );
-is( $r->header_in( "Content-Type" ), "text/plain", "Error content type" );
+is( $r->status, $r->HTTP_METHOD_NOT_ALLOWED, "Service document needs GET." );
 
 do_request( GET => $u->atom_service_document, authenticate => 1 );
-is( $r->status, $r->OK, "Got service document." );
+is( $r->status, $r->HTTP_OK, "Got service document." );
 like( $r->header_in( "Content-Type" ), qr#^\Qapplication/atomsvc+xml\E#, "Service content type" );
 my $service_document_xml = $r->response_content;
 
@@ -146,11 +146,10 @@ is( $r->status, $r->HTTP_UNAUTHORIZED, "
 is( $r->status, $r->HTTP_UNAUTHORIZED, "Categories document protected by authorization." );
 
 do_request( POST => $u->atom_base . "/entries/tags", authenticate => 1 );
-is( $r->status, $r->NOT_FOUND, "Categories document needs GET." );
-is( $r->header_in( "Content-Type" ), "text/plain", "Error content type" );
+is( $r->status, $r->HTTP_METHOD_NOT_ALLOWED, "Categories document needs GET." );
 
 do_request( GET => $u->atom_base . "/entries/tags", authenticate => 1 );
-is( $r->status, $r->OK, "Got categories document." );
+is( $r->status, $r->HTTP_OK, "Got categories document." );
 like( $r->header_in( "Content-Type" ), qr#^\Qapplication/atomcat+xml\E#, "Categories document type" );
 my $categories_document_xml = $r->response_content;
 
@@ -231,7 +230,7 @@ is( $r->status, $r->HTTP_UNAUTHORIZED, "
 is( $r->status, $r->HTTP_UNAUTHORIZED, "Entries feed needs authorization." );
 
 do_request( GET => $u->atom_base . "/entries", authenticate => 1 );
-is( $r->status, $r->OK, "Retrieved entry list" );
+is( $r->status, $r->HTTP_OK, "Retrieved entry list" );
 is( $r->header_in( "Content-Type" ), "application/atom+xml", "AtomAPI entry content type" );
 
 my $feed = XML::Atom::Feed->new( \ $r->response_content );
@@ -247,10 +246,10 @@ is( $r->content_type, "text/plain", "Ato
 is( $r->content_type, "text/plain", "AtomAPI entry content type" );
 
 do_request( POST => $u->atom_base . "/entries/1", authenticate => 1 );
-is( $r->status, $r->NOT_FOUND, $u->atom_base . "/entries/1 does not support POST." );
+is( $r->status, $r->HTTP_METHOD_NOT_ALLOWED, $u->atom_base . "/entries/1 does not support POST." );
 
 do_request( GET => $u->atom_base . "/entries/1", authenticate => 1 );
-is( $r->status, $r->OK, "Retrieved entry" );
+is( $r->status, $r->HTTP_OK, "Retrieved entry" );
 is( $r->content_type, "application/atom+xml", "AtomAPI entry content type" );
 
 $atom_entry_server = XML::Atom::Entry->new( \ $r->response_content );
@@ -293,7 +292,7 @@ foreach my $tag ( @tags ) {
 # put a little bit of time between publish and update
 sleep( 1 );
 do_request( PUT => $u->atom_base . "/entries/1", authenticate => 1, data => { input => $atom_entry->as_xml } );
-is( $r->status, $r->OK, "Edited entry" );
+is( $r->status, $r->HTTP_OK, "Edited entry" );
 is( $r->content_type, "application/atom+xml", "AtomAPI entry content type" );
 
 do_request( GET => $u->atom_base . "/entries/1", authenticate => 1 );
@@ -317,7 +316,7 @@ is( $r->status, $r->HTTP_BAD_REQUEST, "M
 
 
 do_request( DELETE => $u->atom_base . "/entries/1", authenticate => 1 );
-is( $r->status, $r->OK, "Deleted entry" );
+is( $r->status, $r->HTTP_OK, "Deleted entry" );
 $entry_obj = LJ::Entry->new( $u, jitemid => 1 );
 isnt( $entry_obj->valid, "Entry confirmed deleted" );
 
@@ -349,7 +348,7 @@ note( "Checking community functionality.
 
     # community you aren't a member of
     do_request( GET => $nonmemberof_cu->atom_service_document, authenticate => 1 );
-    is( $r->status, $r->OK, "Not a member of the community, but we still get the service document for the user (which doesn't contain the community)." );
+    is( $r->status, $r->HTTP_OK, "Not a member of the community, but we still get the service document for the user (which doesn't contain the community)." );
 
     SKIP: {
         skip "No XML::Atom::Service/XML::Atom::Categories module installed.", 3
@@ -368,7 +367,7 @@ note( "Checking community functionality.
 
     # community you are a member of
     do_request( GET => $memberof_cu->atom_service_document, authenticate => 1 );
-    is( $r->status, $r->OK, "Got service document." );
+    is( $r->status, $r->HTTP_OK, "Got service document." );
     like( $r->header_in( "Content-Type" ), qr#^\Qapplication/atomsvc+xml\E#, "Service content type" );
 
     SKIP: {
@@ -447,7 +446,7 @@ note( "Checking community functionality.
 
     # community you have posting access to
     do_request( GET => $memberof_cu->atom_base . "/entries", authenticate => 1 );
-    is( $r->status, $r->OK, "Retrieved entry list" );
+    is( $r->status, $r->HTTP_OK, "Retrieved entry list" );
     is( $r->header_in( "Content-Type" ), "application/atom+xml", "AtomAPI entry content type" );
 
     my $feed = XML::Atom::Feed->new( \ $r->response_content );
@@ -470,7 +469,7 @@ note( "Checking community functionality.
     # edit (should succeed)
     # delete (should succeed)
     do_request( GET => $memberof_cu->atom_base . "/entries/1", authenticate => 1 );
-    is( $r->status, $r->OK, "Retrieved entry" );
+    is( $r->status, $r->HTTP_OK, "Retrieved entry" );
     is( $r->content_type, "application/atom+xml", "AtomAPI entry content type" );
 
     $atom_entry_server = XML::Atom::Entry->new( \ $r->response_content );
@@ -493,7 +492,7 @@ note( "Checking community functionality.
 
     do_request( PUT => $memberof_cu->atom_base . "/entries/1", authenticate => 1, data => { input => $atom_entry->as_xml } );
 
-    is( $r->status, $r->OK, "Edited entry" );
+    is( $r->status, $r->HTTP_OK, "Edited entry" );
     is( $r->content_type, "application/atom+xml", "AtomAPI entry content type" );
 
     do_request( GET => $memberof_cu->atom_base . "/entries/1", authenticate => 1 );
@@ -510,7 +509,7 @@ note( "Checking community functionality.
 
 
     do_request( DELETE => $memberof_cu->atom_base . "/entries/1", authenticate => 1 );
-    is( $r->status, $r->OK, "Deleted entry" );
+    is( $r->status, $r->HTTP_OK, "Deleted entry" );
     $entry_obj = LJ::Entry->new( $memberof_cu, jitemid => 1 );
     isnt( $entry_obj->valid, "Entry confirmed deleted" );
 
@@ -545,14 +544,14 @@ note( "Checking community functionality.
     # edit (should fail)
     # delete (should succeed)
     do_request( GET => $memberof_cu->atom_base . "/entries/2", authenticate => 1, remote => $admin_u );
-    is( $r->status, $r->OK, "Retrieved entry" );
+    is( $r->status, $r->HTTP_OK, "Retrieved entry" );
     is( $r->content_type, "application/atom+xml", "AtomAPI entry content type" );
 
     do_request( PUT => $memberof_cu->atom_base . "/entries/2", authenticate => 1, remote => $admin_u, data => { input => $atom_entry->as_xml } );
     is( $r->status, $r->HTTP_UNAUTHORIZED, "You don't own this entry (admin_u, edit)" );
 
     do_request( DELETE => $memberof_cu->atom_base . "/entries/2", authenticate => 1, remote => $admin_u );
-    is( $r->status, $r->OK, "Deleted entry (admin_u, delete)" );
+    is( $r->status, $r->HTTP_OK, "Deleted entry (admin_u, delete)" );
     $entry_obj = LJ::Entry->new( $memberof_cu, jitemid => 2 );
     isnt( $entry_obj->valid, "Entry confirmed deleted" );
 }
--------------------------------------------------------------------------------