Bug #58728

Regression: unaccessible protected section with shortcut in rootline

Added by Alexander Stehlik over 5 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
Frontend
Target version:
-
Start date:
2014-05-12
Due date:
% Done:

100%

TYPO3 Version:
6.2
PHP Version:
5.4
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

Description

The fix for #16472 introduced a regression when you have a shortcut in the rootline.

My rootline looks like this:

- Root page (is_siteroot flag is set here)
-- Shortcut to Homepage
--- Homepage
--- Protected page (expand to subpages)
---- Protected Subpage

When I try to access any protected subpage I will always be redirected to the Hompage.

The reason for this seems to be the introduced recursive call to $this->getPageAndRootline() which will set the $this->originalShortcutPage class property which will end up in a redirection to the shortcut.


Related issues

Related to TYPO3 Core - Bug #16472: Non accessible Page And PageNotFound Handler. Closed 2006-08-15
Related to TYPO3 Core - Bug #23178: Wrong HTTP headers sent when trying to access pages that require login Closed 2010-07-14
Related to TYPO3 Core - Bug #18754: improved 404 pagenotfound_handling not working for certain requested URLs/resources Closed 2008-05-06
Related to TYPO3 Core - Feature #17794: add an error code to pageNotFoundAndExit Closed 2007-11-15

Associated revisions

Revision c017900f (diff)
Added by Helmut Hummel over 5 years ago

Revert "[BUGFIX] Inaccessible pages on shortcuts/PageNotFound handler"

This introduced a regression. It turns out that it needs more work
to get all cases covered correctly.

Resolves: #58728
Reverts: #16472
Releases: 6.1, 6.2, 6.3
This reverts commit 9ab3b9b5dd96ae0f955277a8997abb4bd69a66ff

Change-Id: I395e052c1f31adde715f5a25f9d1716c092dd908
Reviewed-on: https://review.typo3.org/31039
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel

Revision 10d47935 (diff)
Added by Helmut Hummel over 5 years ago

Revert "[BUGFIX] Inaccessible pages on shortcuts/PageNotFound handler"

This introduced a regression. It turns out that it needs more work
to get all cases covered correctly.

Resolves: #58728
Reverts: #16472
Releases: 6.1, 6.2, 6.3
This reverts commit 9ab3b9b5dd96ae0f955277a8997abb4bd69a66ff

Change-Id: I395e052c1f31adde715f5a25f9d1716c092dd908
Reviewed-on: https://review.typo3.org/31045
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel

Revision 62692580 (diff)
Added by Helmut Hummel over 5 years ago

Revert "[BUGFIX] Inaccessible pages on shortcuts/PageNotFound handler"

This introduced a regression. It turns out that it needs more work
to get all cases covered correctly.

Resolves: #58728
Reverts: #16472
Releases: 6.1, 6.2, 6.3

This reverts commit 203c1eb9d1c621726be57268619b7b48ffbffeb6

Change-Id: I469461a042f37757daafbf23d27a73ad236fcb50
Reviewed-on: https://review.typo3.org/31041
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel

History

#1 Updated by Alexander Opitz over 5 years ago

What happened till yet on your setup?

#2 Updated by Alexander Stehlik over 5 years ago

Before the page not found handler was triggered when I was not logged in and tried to access the page.

The problem does not seem to occur when I'm logged in.

#3 Updated by Alexander Opitz over 5 years ago

  • Status changed from New to Accepted

So it didn't worked as expected before. But everyone thought this is the correct behavior. ;)

As already written in #16472, this fields needs a complete rethinking. The status vars and the found page are coupled yet and the NotFound handler don't know what to do. Also you can't configure it like you want it.

So in question:

- Revert the fix and work out a better (and cleaner) solution
- Let the patch in and work out a better solution
- Fix it and get new regressions till end of live
- Define the yet existing handling as correct (which seams isn't)

#4 Updated by Alexander Stehlik over 5 years ago

Well, the current behavior would be correct, if the default page not found handler is used. But when I disable the default behavior (redirecting to next accessible page in page tree) this behavior is definitely wrong because the error handler is not triggered at all.

I can't decide which is the best solution. I'm currently reverting the patch in the source I'm using because I never was affected by the bug that is fixed with the patch. But as it looks to me the whole page permission handling could use some love (also performance wise, e.g. #57953).

#5 Updated by Alexander Opitz over 5 years ago

@Alex

"But when I disable the default behavior ..." how do you disable the default behavior?

#6 Updated by Alexander Stehlik over 5 years ago

You can set it in $TYPO3_CONF_VARS['FE']['pageNotFound_handling']. When this setting is empty (as in DefaultConfiguration.php) the user will be redirected to the next upper page.

When you set it to TRUE you will get an error message, that the requested page is not accessible instead. You can also use different prefixes (like REDIRECT: or READFILE:).

#7 Updated by Alexander Opitz over 5 years ago

@Alex

"When this setting is empty the user will be redirected to the next upper page" ... the page was searched but $this->pageNotFound wasn't reseted => Uptree page shown but symlink pages not resolved (1)
"When you set it to TRUE you will get an error message, that the requested page is not accessible instead." ... another page is searched uptree but handler is messaged (2)

Now I remember my old issue: The fix I introduced was to have the handler active but also have the uptree functionality. (3)

So there are 3 things.

(1) Is fixed.
(2) Can be fixed, cause we don't need uptree search if we will error.
(3) Was fixed but resulted in this regression.

So we need a way to enable/disable the behavior of (3) with default disabled if $TYPO3_CONF_VARS['FE']['pageNotFound_handling'] isn't empty.

Does this sounds correct?

#8 Updated by Alexander Opitz over 5 years ago

If we want to fix/rewrite this, we should also consider #23178

#9 Updated by Alexander Opitz over 5 years ago

Hi Ernesto,

how would you like to progress in this regression?

1) Revert patch.
2) Add a possibility to enable/disable the "uptree" handling.

I'd also like to rewrite this part, with backward compatible handling, can this be done for 6.2.x or only for 6.2+x?

#10 Updated by Oliver Hader over 5 years ago

Alexander Opitz wrote:

1) Revert patch.
2) Add a possibility to enable/disable the "uptree" handling.

I'd opt for reverting the change in 6.2 and 6.1 and then work out a clean and consistent solution.
Besides that, I'd consider #23178 as separate issue.

#11 Updated by Gerrit Code Review over 5 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/31039

#12 Updated by Gerrit Code Review over 5 years ago

Patch set 1 for branch TYPO3_6-1 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/31041

#13 Updated by Gerrit Code Review over 5 years ago

Patch set 2 for branch TYPO3_6-1 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/31041

#14 Updated by Gerrit Code Review over 5 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/31039

#15 Updated by Helmut Hummel over 5 years ago

Oliver Hader wrote:

Alexander Opitz wrote:

1) Revert patch.
2) Add a possibility to enable/disable the "uptree" handling.

I'd opt for reverting the change in 6.2 and 6.1 and then work out a clean and consistent solution.
Besides that, I'd consider #23178 as separate issue.

I agree. Reverts are pending now.

If nobody objects, I will merge within the next days...

#16 Updated by Gerrit Code Review over 5 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/31039

#17 Updated by Gerrit Code Review over 5 years ago

Patch set 3 for branch TYPO3_6-1 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/31041

#18 Updated by Gerrit Code Review over 5 years ago

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/31045

#19 Updated by Helmut Hummel over 5 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#20 Updated by Martin Kutschker over 5 years ago

According to http://wiki.typo3.org/TYPO3_CMS_6.1.10 the regresion has not been fixed for 6.1. branch. The way the changelog is written, the regression has been introduced in 6.1.10.

#21 Updated by Alexander Opitz over 5 years ago

@Martin Kuschker

From the 6.1.10 ChangeLog

2014-06-22 6269258 #58728 Revert "[BUGFIX] Inaccessible pages on shortcuts/PageNotFound handler" (Helmut Hummel)
2014-05-22 203c1eb #16472 [BUGFIX] Inaccessible pages on shortcuts/PageNotFound handler (Alexander Opitz)
<<<

So it was introduced and reverted while developing 6.1.10, so no issue here.

#22 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF