[#18509] - Return error 404 instead of error 0 for non-existent page, ex index.php?Itemid=9999
- Fixed in Code Base
- 7 Nov 2017
- Medium
- Build: staging
- # 18509
- Diff
- csthomas:getcomponent
- Success hound No violations found. Woof! Details
- Pending continuous-integration/drone/pr this build is pending Details
User tests: Successful: Unsuccessful:
Fix bad effect of merged PR #18308
Summary of Changes
If the menu item does not exist for a link like index.php?Itemid=9999 then joomla returns
Call to a member function getParams() on null .../libraries/src/Component/ComponentHelper.php:103
instead of returns only error 404
Testing Instructions
Install joomla 3.8.2 RC.
Before the patch, go to the non-existent menu item, such as index.php?Itemid=9999
After patch you should see only 404 error, which is correct.
Expected result
As described above.
Actual result
As described above.
Documentation Changes Required
No
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Title |
|
||||||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18509.
@izharaazmi
can you check urgently please?
| Status | Pending | ⇒ | Ready to Commit |
RTC after two successful tests.
This is an issue, but for me this doesn't sound the right fix. I'd explain.
@franz-wohlkoenig
Let's take off RTC for now until @izharaazmi posts more.
| Status | Ready to Commit | ⇒ | Pending |
I'd suggest this patch.
diff --git a/libraries/src/Component/ComponentHelper.php b/libraries/src/Component/ComponentHelper.php
--- a/libraries/src/Component/ComponentHelper.php
+++ b/libraries/src/Component/ComponentHelper.php
@@ -41,9 +41,11 @@ class ComponentHelper
*/
public static function getComponent($option, $strict = false)
{
- if (isset(static::$components[$option]) || static::load($option))
+ $components = static::getComponents();
+
+ if (isset($components[$option]))
{
- $result = static::$components[$option];
+ $result = $components[$option];
}
else
{
At first I had the same idea, but do we care about testing the units of coverage php?
After this changes unit tests won't test method load() with real $option like 'com_content'?
If this is not a problem the I will change it.
About the current PR, the problem is that it would still cause a B/C check if the $option is null. We should only check deprecation if it's passed. Absence of this parameter is what we are trying to encourage by deprecation. Still allowing '*' is bcoz of the B/C constraints.
I don't see a reason why Tests would fail. If thats the case, we may need to fix it separately. Can you please show me what error you find with the tests?
The code in if (isset($option) && $option != '*') {...} won't be covered in unit tests.
Then OK I will prepare a new version.
Please test again.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18509.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18509.
| Status | Pending | ⇒ | Ready to Commit |
RTC. Thanks folks.
@mbabker Good to go in 3.8.2
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18509.
mbabker
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-11-07 11:40:26 |
| Closed_By | ⇒ | mbabker | |
| Labels |
Added:
?
|
||
I have tested this item✅ successfully on f281ed5
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18509.