You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by ma...@redhat.com on 2012/12/04 13:23:08 UTC

[PATCH 2/2] CIMI - collections use grab_content_type helper to guess type if not supplied

From: marios <ma...@redhat.com>


Signed-off-by: marios <ma...@redhat.com>
---
 server/lib/cimi/collections/addresses.rb         |  2 +-
 server/lib/cimi/collections/credentials.rb       |  2 +-
 server/lib/cimi/collections/machine_templates.rb |  2 +-
 server/lib/cimi/collections/machines.rb          | 10 +++++-----
 server/lib/cimi/collections/network_ports.rb     |  6 +++---
 server/lib/cimi/collections/networks.rb          |  8 ++++----
 6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/server/lib/cimi/collections/addresses.rb b/server/lib/cimi/collections/addresses.rb
index a5a5986..6f016b2 100644
--- a/server/lib/cimi/collections/addresses.rb
+++ b/server/lib/cimi/collections/addresses.rb
@@ -47,7 +47,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_address do
         description "Create a new Address"
         control do
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             address = CIMI::Model::Address.create(request.body.read, self, :json)
           else
             address = CIMI::Model::Address.create(request.body.read, self, :xml)
diff --git a/server/lib/cimi/collections/credentials.rb b/server/lib/cimi/collections/credentials.rb
index 24c66c0..f2ca059 100644
--- a/server/lib/cimi/collections/credentials.rb
+++ b/server/lib/cimi/collections/credentials.rb
@@ -46,7 +46,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_key do
         description "Show specific machine admin"
         control do
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             new_admin = Credential.create_from_json(request.body.read, self)
           else
             new_admin = Credential.create_from_xml(request.body.read, self)
diff --git a/server/lib/cimi/collections/machine_templates.rb b/server/lib/cimi/collections/machine_templates.rb
index bda57a0..a90f360 100644
--- a/server/lib/cimi/collections/machine_templates.rb
+++ b/server/lib/cimi/collections/machine_templates.rb
@@ -45,7 +45,7 @@ module CIMI::Collections
       operation :create do
         description "Create new machine template"
         control do
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             new_machine_template = CIMI::Model::MachineTemplate.create_from_json(request.body.read, self)
           else
             new_machine_template = CIMI::Model::MachineTemplate.create_from_xml(request.body.read, self)
diff --git a/server/lib/cimi/collections/machines.rb b/server/lib/cimi/collections/machines.rb
index 20fcfe0..91fbc64 100644
--- a/server/lib/cimi/collections/machines.rb
+++ b/server/lib/cimi/collections/machines.rb
@@ -46,7 +46,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_instance do
         description "Create a new Machine entity."
         control do
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             new_machine = Machine.create_from_json(request.body.read, self)
           else
             new_machine = Machine.create_from_xml(request.body.read, self)
@@ -73,7 +73,7 @@ module CIMI::Collections
         param :id,          :string,    :required
         control do
           machine = Machine.find(params[:id], self)
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -90,7 +90,7 @@ module CIMI::Collections
         param :id,          :string,    :required
         control do
           machine = Machine.find(params[:id], self)
-          if request.content_type.end_with?("json")
+          if  grab_content_type(request.content_type, request.body) == :json
             action = Action.from_json(request.body.read.gsub("restart", "reboot"))
           else
             action = Action.from_xml(request.body.read.gsub("restart", "reboot"))
@@ -107,7 +107,7 @@ module CIMI::Collections
         param :id,          :string,    :required
         control do
           machine = Machine.find(params[:id], self)
-          if request.content_type.end_with?("json")
+          if  grab_content_type(request.content_type, request.body) == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -175,7 +175,7 @@ module CIMI::Collections
         description "Attach CIMI Volume(s) to a machine."
         param :id,          :string,    :required
         control do
-          if request.content_type.end_with?("json")
+          if  grab_content_type(request.content_type, request.body) == :json
             volume_to_attach, location = MachineVolume.find_to_attach_from_json(request.body.read, self)
           else
             volume_to_attach, location = MachineVolume.find_to_attach_from_xml(request.body.read, self)
diff --git a/server/lib/cimi/collections/network_ports.rb b/server/lib/cimi/collections/network_ports.rb
index 1eb71c6..9a009d6 100644
--- a/server/lib/cimi/collections/network_ports.rb
+++ b/server/lib/cimi/collections/network_ports.rb
@@ -47,7 +47,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_network_port do
         description "Create a new NetworkPort"
         control do
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             network_port = CIMI::Model::NetworkPort.create(request.body.read, self, :json)
           else
             network_port = CIMI::Model::NetworkPort.create(request.body.read, self, :xml)
@@ -73,7 +73,7 @@ module CIMI::Collections
         control do
           network_port = NetworkPort.find(params[:id], self)
           report_error(404) unless network_port
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -91,7 +91,7 @@ module CIMI::Collections
         control do
           network_port = NetworkPort.find(params[:id], self)
           report_error(404) unless network_port
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
diff --git a/server/lib/cimi/collections/networks.rb b/server/lib/cimi/collections/networks.rb
index 002b828..6500c82 100644
--- a/server/lib/cimi/collections/networks.rb
+++ b/server/lib/cimi/collections/networks.rb
@@ -46,7 +46,7 @@ module CIMI::Collections
       operation :create, :with_capability => :create_network do
         description "Create a new Network"
         control do
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             network = Network.create(request.body.read, self, :json)
           else
             network = Network.create(request.body.read, self, :xml)
@@ -72,7 +72,7 @@ module CIMI::Collections
         control do
           network = Network.find(params[:id], self)
           report_error(404) unless network
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -90,7 +90,7 @@ module CIMI::Collections
         control do
           network = Network.find(params[:id], self)
           report_error(404) unless network
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
@@ -108,7 +108,7 @@ module CIMI::Collections
         control do
           network = Network.find(params[:id], self)
           report_error(404) unless network
-          if request.content_type.end_with?("json")
+          if grab_content_type(request.content_type, request.body) == :json
             action = Action.from_json(request.body.read)
           else
             action = Action.from_xml(request.body.read)
-- 
1.7.11.7


Re: [PATCH 2/2] CIMI - collections use grab_content_type helper to guess type if not supplied

Posted by "marios@redhat.com" <ma...@redhat.com>.
On 05/12/12 02:45, David Lutterkort wrote:
> On Tue, 2012-12-04 at 14:23 +0200, marios@redhat.com wrote:
>> From: marios <ma...@redhat.com>
>>
>>
>> Signed-off-by: marios <ma...@redhat.com>
>> ---
>>  server/lib/cimi/collections/addresses.rb         |  2 +-
>>  server/lib/cimi/collections/credentials.rb       |  2 +-
>>  server/lib/cimi/collections/machine_templates.rb |  2 +-
>>  server/lib/cimi/collections/machines.rb          | 10 +++++-----
>>  server/lib/cimi/collections/network_ports.rb     |  6 +++---
>>  server/lib/cimi/collections/networks.rb          |  8 ++++----
>>  6 files changed, 15 insertions(+), 15 deletions(-)
> 
> I think what grab_content_type is doing is overly lenient - we should
> reject requests that have a content-type other than application/xml or
> application/json with a 415. Note that the comparison of the
> Content-Type header with the above two values must be case-insensitive.
> 

ok - I added this more for personal comfort - like when I forgot to add
the 'Content-Type' header to my cURL command. Digging a little deeper,
the CIMI spec defers to RFC 2616 and looking at that Content-Type is a
'should' rather than a 'must'. I enforced the strict 'xml or json only'
check by removing the call to 'guess_content_type' and raise a 415
appropriately.

I also added the unit test (as part of part_4test.rb). Patches are at
http://tracker.deltacloud.org/set/181

thanks, marios


> David
> 
> 


Re: [PATCH 2/2] CIMI - collections use grab_content_type helper to guess type if not supplied

Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2012-12-04 at 14:23 +0200, marios@redhat.com wrote:
> From: marios <ma...@redhat.com>
> 
> 
> Signed-off-by: marios <ma...@redhat.com>
> ---
>  server/lib/cimi/collections/addresses.rb         |  2 +-
>  server/lib/cimi/collections/credentials.rb       |  2 +-
>  server/lib/cimi/collections/machine_templates.rb |  2 +-
>  server/lib/cimi/collections/machines.rb          | 10 +++++-----
>  server/lib/cimi/collections/network_ports.rb     |  6 +++---
>  server/lib/cimi/collections/networks.rb          |  8 ++++----
>  6 files changed, 15 insertions(+), 15 deletions(-)

I think what grab_content_type is doing is overly lenient - we should
reject requests that have a content-type other than application/xml or
application/json with a 415. Note that the comparison of the
Content-Type header with the above two values must be case-insensitive.

David