You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "Sarat Pediredla (JIRA)" <ji...@apache.org> on 2008/02/27 13:43:08 UTC
[jira] Created: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
REST plugin does not throw a 404 if resource cannot be found
------------------------------------------------------------
Key: WW-2515
URL: https://issues.apache.org/struts/browse/WW-2515
Project: Struts 2
Issue Type: Bug
Components: Plugin - REST
Affects Versions: 2.1.1
Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
Reporter: Sarat Pediredla
When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Jeromy Evans (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43670#action_43670 ]
Jeromy Evans commented on WW-2515:
----------------------------------
Yeah, i agree this can go to Will not fix.
The developer should return a new HttpHeaders("error").wihStatus(404). That allows them to map/prepare an appropriate error result and for the ContentTypeDeterminer to do its work.
Developers can also catch the MethodNotFound exception and use a global exception mapping to return a 405 instead of 500. I'm not sure how the ContentTypeDeterminer can ensure it doesn't return an html error page for an xml/json/other content type though.
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Fix For: 2.1.2
>
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Sarat Pediredla (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43408#action_43408 ]
Sarat Pediredla commented on WW-2515:
-------------------------------------
In follow up to that, I dont think the REST framework even throws a 405 "method not supported". All it does is throw a 500 - Internal Server Error with a NoSuchMethodException which isnt entirely RESTful. A client like curl will only see a 500 exception in the response with whatever the configured result handler is.
I genuinely think (although this is up for debate) that the REST framework should be responsible for returning the right HTTP Status Codes rather than leave it to the clients to handle exceptions.
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Sarat Pediredla (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43407#action_43407 ]
Sarat Pediredla commented on WW-2515:
-------------------------------------
The method not supported solution only works for when the resource does not have a corresponding method for the operation/verb requested.
I think there is a slight confusion here in the definition of "resource". I meant it in the RESTful way although I agree that accessing an invalid action will throw a 404. The problem is more a REST one. Other RESTful frameworks (like RoR) implement the same concept of throwing a 404 if an "object or resource" you requested does not exist. In terms of Struts 2 REST, an action will always by default map to a "list" of objects (i.e. index() method) rather than the specific resource (i.e. show()).
If the controller is non-ModelDriven, then would that not invalidate any other real use of the REST plugin as handler.toObject wouldnt return a true representation of the object representing the resource but just a representation of the action (which in most circumstance is just a RESTful controller). In those cases, you wouldnt want a 404 returned and rightly so?
As for the appropriateness, the 404 signifies that the "resource cannot be found" and I think this is definitely relevant as far as REST goes.
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Don Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Don Brown updated WW-2515:
--------------------------
Fix Version/s: 2.1.2
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Fix For: 2.1.2
>
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Sarat Pediredla (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sarat Pediredla updated WW-2515:
--------------------------------
Attachment: WW-2515.patch
This patch will ensure that the REST framework returns a 404 error code if a requested resource cannot be found.
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Don Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43668#action_43668 ]
Don Brown commented on WW-2515:
-------------------------------
Another idea is to simply set the 404 status on the HttpHeaders object, which should result in the correct outcome. I can see the advantage of exceptions, as it makes the flow in the Action code easier, however, so that may be the best solution in the end.
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Fix For: 2.1.2
>
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Sarat Pediredla (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43419#action_43419 ]
Sarat Pediredla commented on WW-2515:
-------------------------------------
I agree with your comments. I think this is a tricky one overall. In RoR (in production mode), as far as I can remember, a missing controller raises a 404.
I also agree that maybe the ContenTypeInterceptor is not the right place to throw the 404. Maybe the patch was hasty and in reconsideration I dont entirely like the way it closes the response. Could you please remove the current patch?
Maybe the best way is to define a ResourceNotFound exception that gets thrown by an action (after all, the action is best placed to know whether the resource exists or not) and then trap this in the plugin and convert it into HTTP Status Code 404? Likewise, with MethodNotSupported? Currently, I am sure an IllegalArgumentException gets thrown instead of a status code when in REST mode.
If this is agreeable, I will create another patch for that solution.
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (WW-2515) REST plugin does not throw a
404 if resource cannot be found
Posted by "Sarat Pediredla (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43407#action_43407 ]
sarat.pediredla edited comment on WW-2515 at 2/27/08 5:45 AM:
--------------------------------------------------------------
Other RESTful frameworks (like RoR) implement the same concept of throwing a 404 if an "object or resource" you requested does not exist. In terms of Struts 2 REST, an action will always by default map to a "list" of objects (i.e. index() method) rather than the specific resource (i.e. show()).
If the controller is non-ModelDriven, then would that not invalidate any other real use of the REST plugin as handler.toObject wouldnt return a true representation of the object representing the resource but just a representation of the action (which in most circumstance is just a RESTful controller). In those cases, you wouldnt want a 404 returned and rightly so?
As for the appropriateness, the 404 signifies that the "resource cannot be found" and I think this is definitely relevant as far as REST goes.
was (Author: sarat.pediredla):
The method not supported solution only works for when the resource does not have a corresponding method for the operation/verb requested.
I think there is a slight confusion here in the definition of "resource". I meant it in the RESTful way although I agree that accessing an invalid action will throw a 404. The problem is more a REST one. Other RESTful frameworks (like RoR) implement the same concept of throwing a 404 if an "object or resource" you requested does not exist. In terms of Struts 2 REST, an action will always by default map to a "list" of objects (i.e. index() method) rather than the specific resource (i.e. show()).
If the controller is non-ModelDriven, then would that not invalidate any other real use of the REST plugin as handler.toObject wouldnt return a true representation of the object representing the resource but just a representation of the action (which in most circumstance is just a RESTful controller). In those cases, you wouldnt want a 404 returned and rightly so?
As for the appropriateness, the 404 signifies that the "resource cannot be found" and I think this is definitely relevant as far as REST goes.
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Don Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Don Brown resolved WW-2515.
---------------------------
Resolution: Not A Problem
Fix Version/s: (was: 2.1.2)
Assignee: Don Brown
I think we determined there isn't a bug with how things work now. Please open a new feature ticket if you want the addition of exceptions that you can throw to get the 404 automatically. Patches welcome :)
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Assignee: Don Brown
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Jeromy Evans (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43406#action_43406 ]
Jeromy Evans commented on WW-2515:
----------------------------------
I agree checking for a null model is a simple solution, but:
- is it really appropriate to return a 404 even though the action may have executed?
- this solution doesn't work for non-ModelDriven controllers as the target (action) is always non-null
- accessing a resource that doesn't exist (eg. the action doesn't exist) still throws an exception instead of a 404
I expect the solution should be combined with the one that generates a 405 "method not supported" where the action doesn't have the corresponding method (currently throws a method not found exception). This may be best handled by default exception handlers but I'm not sure.
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WW-2515) REST plugin does not throw a 404 if
resource cannot be found
Posted by "Jeromy Evans (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/struts/browse/WW-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43416#action_43416 ]
Jeromy Evans commented on WW-2515:
----------------------------------
Last comment agreed. That's why I suspect there should be a ResourceNotFound and MethodNotSupported exception that can be raised either by the RestActionMapper or Action. A default exception handler could return 404 or 405 respectively. I think returning 404's is part of a larger solution rather than this patch.
For non-ModelDriven controllers, the action is serialized for the result and the action's properties define the model. In this case a 404 may still be an appropriate response even though the target seen by the ContentTypeInterceptor is non-null. I also still have a concern about responding with a 404 after the action has executed (I can see support about an action being invoked by a browser saying it couldn't find it).
I understand in RoR that "GET /path/user/1" returns a 404 if that id is invalid
What happens in RoR if '/path/user' does not exist as a controller? is that a 404 too?
> REST plugin does not throw a 404 if resource cannot be found
> ------------------------------------------------------------
>
> Key: WW-2515
> URL: https://issues.apache.org/struts/browse/WW-2515
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Affects Versions: 2.1.1
> Environment: Struts 2.1.1-SNAPSHOT using Jetty 6.1.5
> Reporter: Sarat Pediredla
> Attachments: WW-2515.patch
>
>
> When there is a GET request for a resource that does not exist, example GET /authors/25 where author with id 25 does not exist, the getModel() method of the action should return null (and in most cases will return null as the object does not exist).
> However, the current ContentTypeInterceptor implementation is not entirely RESTful as the right response should be HTTP Status Code 404 "Not Found".
> I have patched the ContentTypeInterceptor to return a 404 status if the requested resource could not be found.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.