Bug #54217

Using "(/)" as defaultUriSuffix only works for top-level pages

Added by Andreas Wolf over 1 year ago. Updated over 1 year ago.

Status:Closed Start date:2013-12-05
Priority:Should have Due date:
Assigned To:Bastian Waidelich % Done:

0%
Category:-
Target version:-

Description

We've got an existing site on Neos that used to have URLs ending in "/". When upgrading to a recent Neos master (about two weeks ago), the URLs changed to ".html". On 1.0 Beta 2, we have set "defaultUriSuffix" in Routes.yaml to "(/)" ("/" was not accepted because the routing mechanism generally does not like routes ending in a slash).

This works fine for pages directly beneath the site root, but pages inside a subpage are not found. The system log says something like this:

13-12-05 17:02:23 32682 NOTICE Flow Router route(): No route matched the route path "subpage/subsubpage/". 13-12-05 17:02:23 32682 DEBUG Flow No CSRF required, safe request 13-12-05 17:02:23 32682 CRITICAL Flow Uncaught exception #1303209195 in line 74 of /data/www/mysite/Data/Temporary/Development/SubContextThinkpad/Cache/Code/Flow_Object_Classes/TYPO3_Flow_Mvc_Dispatcher.php: No controller could be resolved which would match your request. Package key: "", controller name: "Standard". (GET http://dev.mysite.localsite/subpage/subsubpage/) - See also: 2013120517022324e652.txt

I'm a bit clueless where to exactly look for this bug. When setting defaultUriSuffix to ".html", "http://dev.mysite.localsite/subpage/subsubpage.html" is found.

54217.diff Magnifier (708 Bytes) Andreas Wolf, 2013-12-05 22:23

Related issues

related to TYPO3.Neos - Bug #55492: Disabling the defaultUriSuffix breaks routing Resolved 2014-01-31
related to TYPO3.Neos - Bug #54187: Router ignores default Suffix from browser Resolved 2013-12-04

Associated revisions

Revision eaaa9aef
Added by Bastian Waidelich over 1 year ago

[!!!][BUGFIX] Only accepts URLs with the configured URI suffix

This change removes two obsolete routes and adjusts the
``FrontendNodeRoutePartHandler`` so that it matches the full node path.

Background:
The route "Content with some format" accepted all URLs in the format
``{node}.{@format}`` which lead to duplicate content (if your default
suffix is empty and the user appends ".html") and weird errors (if an
arbitrary extension is added to the path, like "some/path.anything").

Besides this change simplifies the ``FrontendNodeRoutePartHandler`` so
that it does not ignore the ``splitString`` option if no dot could be
found in the request path.

This is a breaking change if you relied on the fact that any format
suffix is accepted by Neos.
If you provide a custom format (for example "ics") you should register
a custom route for it in your package::

-
    name:  'ICS'
    uriPattern: '{node}.ics'
    defaults:
      '@package':    'TYPO3.Neos'
      '@controller': 'Frontend\Node'
      '@action':     'show'
      '@format':     'ics'
    routeParts:
      'node':
        handler:     'TYPO3\Neos\Routing\FrontendNodeRoutePartHandler'

Releases: master, 1.0
Change-Id: Iea1b92b07daba69f5f76cd7ceb7b81f2d14d68a1
Fixes: #54187
Related: #54217
Related: #55492
Reviewed-on: https://review.typo3.org/25980
Tested-by: Bastian Waidelich
Reviewed-by: Dominique Feyer
Tested-by: Dominique Feyer
Reviewed-by: Aske Ertmann
Reviewed-by: Bastian Waidelich

Revision 437ec25a
Added by Bastian Waidelich over 1 year ago

[!!!][BUGFIX] Only accepts URLs with the configured URI suffix

This change removes two obsolete routes and adjusts the
``FrontendNodeRoutePartHandler`` so that it matches the full node path.

Background:
The route "Content with some format" accepted all URLs in the format
``{node}.{@format}`` which lead to duplicate content (if your default
suffix is empty and the user appends ".html") and weird errors (if an
arbitrary extension is added to the path, like "some/path.anything").

Besides this change simplifies the ``FrontendNodeRoutePartHandler`` so
that it does not ignore the ``splitString`` option if no dot could be
found in the request path.

This is a breaking change if you relied on the fact that any format
suffix is accepted by Neos.
If you provide a custom format (for example "ics") you should register
a custom route for it in your package::

-
    name:  'ICS'
    uriPattern: '{node}.ics'
    defaults:
      '@package':    'TYPO3.Neos'
      '@controller': 'Frontend\Node'
      '@action':     'show'
      '@format':     'ics'
    routeParts:
      'node':
        handler:     'TYPO3\Neos\Routing\FrontendNodeRoutePartHandler'

Releases: master, 1.0
Change-Id: Iea1b92b07daba69f5f76cd7ceb7b81f2d14d68a1
Fixes: #54187
Related: #54217
Related: #55492
Reviewed-on: https://review.typo3.org/27656
Reviewed-by: Bastian Waidelich
Tested-by: Bastian Waidelich
Reviewed-by: Aske Ertmann

History

#1 Updated by Bastian Waidelich over 1 year ago

  • Status changed from New to Needs Feedback
  • Assigned To set to Bastian Waidelich

Hi Andreas,

thanks for the report.
If you want to remove the ".html" suffix replace the variable with an empty string as mentioned here: https://review.typo3.org/#/c/25569/ :

-
  name: 'TYPO3 Neos'
  uriPattern: '<TYPO3NeosSubroutes>'
  subRoutes:
    'TYPO3NeosSubroutes':
      package: 'TYPO3.Neos'
      variables:
        'defaultUriSuffix': ''

Does that work for you?

#2 Updated by Andreas Wolf over 1 year ago

Bastian Waidelich wrote:

Hi Andreas,

thanks for the report.
If you want to remove the ".html" suffix replace the variable with an empty string as mentioned here: https://review.typo3.org/#/c/25569/ :
[...]

Does that work for you?

I just tried that independently from your suggestion, but it does not really fix the problem. Instead of giving a 404, the first level is matched, giving a wrong page. This does not work for nodes that don't exist, but if the desired node exists, a wrong node (directly beneath the top level) is returned.

What I also noticed is that the FrontendNodeRoutePartHandler is called multiple times if the node does not exist (once with the full path, once with only the first level [which is then returned] and then thrice with the full path again).

The reason why my initial attempt did not work is that the "/" is set as the split string in Route.php in method `parse` in the `case self::ROUTEPART_TYPE_STATIC:` block. Therefore, the first part is queried and found, and the latter parts are not found anymore as there is no node with only the subpath. If the search was done recursively, it would probably work – and the correct node is also found, just the matching against the path does not work, because the path is cut off too early.

#3 Updated by Andreas Wolf over 1 year ago

I could fix this, at least for using "(/)" as the defaultUriSuffix. See attached patch for Flow. The trick is to not use / as the split string if that string is the last part in a route.

Downside of this approach is that the route "content with URI suffix and type" is broken afterwards, but this exact route is the reason why "" as the defaultUriSuffix does not work. In my eyes the concept of this route is broken anyways, as there is no way for Neos to determine which strings are allowed as "type". Together with the FrontendNodeRoutePartHandler, which can match any part of a node path (so /foo/bar/baz would match both for the node /foo/bar/baz without a type and the node /foo/bar with type "baz"), this looks like a pretty good road straight to URI hell ;-)

The patch is far from optimal, but for my usecase it fixes the problem.

#4 Updated by Andreas Wolf over 1 year ago

Update No. 2:

For "" as defaultUriSuffix, the problem can also be fixed by changing the order of "content with URI suffix format" and "content with URI suffix and type" in Configuration/Routes.Frontend.yaml. Then the latter route will never match, but the former will match for the correct page (even with a trailing slash in the URL).

Only thing missing is a clarification on why the route with type is needed at all...

#5 Updated by Bastian Waidelich over 1 year ago

Andreas Wolf wrote:

Hi Andreas,

thanks for the elaborate feedback!

Only thing missing is a clarification on why the route with type is needed at all...

It seems this is a left-over that is completely obsolete. The {type} is never used in the code.
Could you please check whether https://review.typo3.org/25980 solves the issue for you?

#6 Updated by Bastian Waidelich over 1 year ago

  • Category deleted (Frontend)

#7 Updated by Bastian Waidelich over 1 year ago

This should be fixed with #55492 - can someone please confirm?

#8 Updated by Bastian Waidelich over 1 year ago

  • Status changed from Needs Feedback to Closed

I close this as duplicate of #54187 for now - feel free to comment/re-open if you don't agree

Also available in: Atom PDF