Log out and enter to administrator/index.php?option=com_xxx&format=json
(GET or POST)
Note: Component and view doesn't need to exist as the request gets intercepted by Joomla.
401 Unauthorized
500 View not found [name, type, prefix]: login, json, loginView
Result should be the same for all non-html views (or as long as loginView doesn't exist).
I know how it works, I just think there should be some error catching in login controller to send out different response. After all it is unauthorized request which cannot be handled by login component.
How about something like this?
public function display($cachable = false, $urlparams = false)
{
/*
* Special treatment is required for this component, as this view may be called
* after a session timeout. We must reset the view and layout prior to display
* otherwise an error will occur.
*/
$this->input->set('view', 'login');
$this->input->set('layout', 'default');
try
{
parent::display();
}
catch (Exception $e)
{
if (!strpos($e->getMessage(), 'loginView'))
{
throw $e;
}
throw new RuntimeException(JText::_('JERROR_ALERTNOAUTHOR'), 401);
}
}
The display task can't do anything useful (login/logout could conceivably). A fix though IMO shouldn't be limited to com_login (maybe if you want to change the error code it'd need to be). Globally you have this issue with all components and throwing a 500 view not found message when it should probably be 404, at least that would be a more correct code for the condition. And even by the HTTP spec 403 would be the preferred code.
I would not suggest string parsing the exception message here. If the checked conditions of the string change, it changes the required checks. If you really want this one case to change, I'd say check the request format and throw your exception before calling the parent display method.
You're right, this would be much better:
public function display($cachable = false, $urlparams = false)
{
/*
* Special treatment is required for this component, as this view may be called
* after a session timeout. We must reset the view and layout prior to display
* otherwise an error will occur.
*/
$this->input->set('view', 'login');
$this->input->set('layout', 'default');
// For non-html formats we do not have login view, so just display 403 instead
if ($this->input->get('format', 'html') !== 'html') {
throw new RuntimeException(JText::_('JERROR_ALERTNOAUTHOR'), 403);
}
parent::display();
}
I wouldn't display 404 code when user hasn't been logged in. All components already show 403 if you do not have permissions to access them, regardless if the view/task exists or not.
Globally IMO a 500 code for controller/view not found isn't right and a 404
should be returned. com_content doesn't support JSON views; a 404 would
better communicate that than 500 which should mean a server error. Also
globally HTML error pages shouldn't be the error response format but that's
a B/C breaking refactoring for another year.
On Tuesday, May 3, 2016, Matias Griese notifications@github.com wrote:
I wouldn't display 404 code when user hasn't been logged in. All
components already show 403 if you do not have permissions to access them,
regardless if the view/task exists or not.—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#10212 (comment)
Oh yes, I must be sleeping as I didn't pay attention that the response was really 500 -- I was just thinking this one case as 404 would be wrong for unauthorized, too.
I agree that globally 500 response is wrong and 404 would be the correct response as those are really pages which do not exist. I also think that its a bug and not API change...
But what about the above change? Its really confusing for users to start getting 500 errors if their session timed out -- 403 errors would allow me to display proper message instead of just displaying the error.
Eh, that'd work.
Category | ⇒ | Administration |
Labels |
Added:
?
|
If the issue is just that we don't have a json view for that page - then I think it is supposed to be a 415 Unsupported Media Type status?
I think it should be consistent with all the components. Each component starts with 403 error check if you don't have access to them -- regardless if the view exists/is supported or not. The only exception is the login screen which appears to the guests, but as login is only supported with html, I would still return 403, which would happen if login didn't intercept the request.
The best reason for returning 403
is that it can then properly be handled inside the client which is making that request. Any other code wouldn't make any sense because of it changes the behaviour of the application. Why would the same request first return 200 OK
and after 15 minutes 415 Unsupported
? It just doesn't make any sense from the application point of view.
I'll find some time to commit this change in a few days. I'm already running my local copy with the change implemented and the error message I get back really makes sense to me.
There is now pull request from this issue. Sorry about the delay.
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-06-21 07:44:34 |
Closed_By | ⇒ | zero-24 |
Closed_Date | 2016-06-21 07:44:34 | ⇒ | 2016-06-21 07:44:35 |
Closed_By | zero-24 | ⇒ | joomla-cms-bot |
Set to "closed" on behalf of @zero-24 by The JTracker Application at issues.joomla.org/joomla-cms/10212
PR is: #10888 Thanks.
Believe it or not 401 is NOT the expected result because you've gotten past all the redirects and ACL checks at that point and Joomla is trying to give the user a login page. And considering Joomla's error handling layer barely handles HTML requests, can't say I'm surprised non-HTML gives something "broken".