From 6ed6781a688b2877507b7d55f5f55a9d83d84fe9 Mon Sep 17 00:00:00 2001
From: Jay Cox <jaycox@electricmessiah.org>
Date: Tue, 20 Jan 2015 23:38:02 -0800
Subject: [PATCH] Disable GET requests changing firewall status

Also return correct status codes.
---
 .../firewall/FirewallDisableResource.java     | 20 +++++++++----------
 .../firewall/FirewallEnableResource.java      | 20 +++++++++----------
 .../firewall/FirewallSubnetMaskResource.java  |  6 ++++++
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/firewall/FirewallDisableResource.java b/src/main/java/net/floodlightcontroller/firewall/FirewallDisableResource.java
index 6f51ebf74..83c612283 100644
--- a/src/main/java/net/floodlightcontroller/firewall/FirewallDisableResource.java
+++ b/src/main/java/net/floodlightcontroller/firewall/FirewallDisableResource.java
@@ -17,8 +17,9 @@
 
 package net.floodlightcontroller.firewall;
 
-import org.restlet.resource.Post;
 import org.restlet.resource.Get;
+import org.restlet.resource.Put;
+import org.restlet.data.Status;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -26,27 +27,26 @@ import org.slf4j.LoggerFactory;
 
 /*
  * Rest API endpoint to disable the firewall
- *
- * Contrary to best practices it changes the state on both GET and POST
- * We should disable this behavior for GET as soon as we can be sure
- * that no clients depend on this behavior.
  */
 public class FirewallDisableResource extends FirewallResourceBase {
     private static final Logger log = LoggerFactory.getLogger(FirewallDisableResource.class);
 
     @Get("json")
     public Object handleRequest() {
-	log.warn("REST call to FirewallDisableResource with method GET is depreciated. Use POST: ");
-	
-	return handlePost();
+        log.warn("call to FirewallDisableResource with method GET is not allowed. Use PUT: ");
+        
+        setStatus(Status.CLIENT_ERROR_METHOD_NOT_ALLOWED);
+	return "{\"status\" : \"failure\", \"details\" : \"Use PUT to disable firewall\"}";
     }
 
-    @Post("json")
-    public Object handlePost() {
+    @Put("json")
+    public Object handlePut() {
         IFirewallService firewall = getFirewallService();
 
 	firewall.enableFirewall(false);
 
+        setStatus(Status.SUCCESS_OK);
+
 	return "{\"status\" : \"success\", \"details\" : \"firewall stopped\"}";
     }
 }
diff --git a/src/main/java/net/floodlightcontroller/firewall/FirewallEnableResource.java b/src/main/java/net/floodlightcontroller/firewall/FirewallEnableResource.java
index 4355db1b1..fa7709681 100644
--- a/src/main/java/net/floodlightcontroller/firewall/FirewallEnableResource.java
+++ b/src/main/java/net/floodlightcontroller/firewall/FirewallEnableResource.java
@@ -17,8 +17,9 @@
 
 package net.floodlightcontroller.firewall;
 
-import org.restlet.resource.Post;
 import org.restlet.resource.Get;
+import org.restlet.resource.Put;
+import org.restlet.data.Status;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -26,27 +27,26 @@ import org.slf4j.LoggerFactory;
 
 /*
  * Rest API endpoint to enable the firewall
- *
- * Contrary to best practices it changes the state on both GET and POST
- * We should disable this behavior for GET as soon as we can be sure
- * that no clients depend on this behavior.
  */
 public class FirewallEnableResource extends FirewallResourceBase {
     private static final Logger log = LoggerFactory.getLogger(FirewallEnableResource.class);
 
     @Get("json")
     public Object handleRequest() {
-	log.warn("REST call to FirewallEnableResource with method GET is depreciated.  Use POST: ");
-	
-	return handlePost();
+        log.warn("call to FirewallDisableResource with method GET is not allowed. Use PUT: ");
+        
+        setStatus(Status.CLIENT_ERROR_METHOD_NOT_ALLOWED);
+	return "{\"status\" : \"failure\", \"details\" : \"Use PUT to enable firewall\"}";
     }
 
-    @Post("json")
-    public Object handlePost() {
+    @Put("json")
+    public Object handlePut() {
         IFirewallService firewall = getFirewallService();
 
 	firewall.enableFirewall(true);
 
+        setStatus(Status.SUCCESS_OK);
+
 	return "{\"status\" : \"success\", \"details\" : \"firewall running\"}";
     }
 }
diff --git a/src/main/java/net/floodlightcontroller/firewall/FirewallSubnetMaskResource.java b/src/main/java/net/floodlightcontroller/firewall/FirewallSubnetMaskResource.java
index f76dd7840..e9ff632c9 100644
--- a/src/main/java/net/floodlightcontroller/firewall/FirewallSubnetMaskResource.java
+++ b/src/main/java/net/floodlightcontroller/firewall/FirewallSubnetMaskResource.java
@@ -26,6 +26,7 @@ import com.fasterxml.jackson.databind.MappingJsonFactory;
 import org.restlet.resource.Post;
 import org.restlet.resource.Get;
 import org.restlet.resource.ServerResource;
+import org.restlet.data.Status;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -53,9 +54,14 @@ public class FirewallSubnetMaskResource extends FirewallResourceBase {
             newMask = jsonExtractSubnetMask(fmJson);
         } catch (IOException e) {
             log.error("Error parsing new subnet mask: " + fmJson, e);
+            setStatus(Status.CLIENT_ERROR_BAD_REQUEST);
             return "{\"status\" : \"Error! Could not parse new subnet mask, see log for details.\"}";
         }
+
         firewall.setSubnetMask(newMask);
+
+        setStatus(Status.SUCCESS_OK);
+
         return ("{\"status\" : \"subnet mask set\"}");
     }
 
-- 
GitLab