[MAGNOLIA-2115] The ServletDispatchingFilter doesn't support the servlet spec for mapping of servlets Created: 18/Apr/08  Updated: 23/Jan/13  Resolved: 16/Sep/08

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 3.5.4
Fix Version/s: 3.6.2, 3.6.3

Type: Improvement Priority: Major
Reporter: Tom Jensen Assignee: Magnolia International
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File servletFilterFix.patch    
Issue Links:
relation
is related to MAGNOLIA-2899 Make ServletDispatchingFilter i18n aware Closed
supersession
supersedes MAGNOLIA-1984 Mappings for wrapped servlets are not... Closed
Template:
Patch included:
Yes
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Testcase included:
Yes
Date of First Response:

 Description   

Proposed implementation:

1) A basic mapping must adhere to the servlet spec exactly (i.e., /test* would require the url passed from the browser to literally be '/test*') servletPath and pathInfo are split according to the spec.
2) A mapping prepended with 'regex:' is treated as a regular expression but the servletPath and pathInfo are more complex:
a) The servletPath will always include everything that is within the mapping url with the following exception
b) If the regex ends with a '/' it is included, along with any other characters to the end of the request url as part of the pathInfo
c) No groupings (parentheses) are allowed unless escaped to be treated as literals

See attached patch for implementation of this with updated tests.



 Comments   
Comment by Magnolia International [ 10/Jul/08 ]

Argh - sorry I didn't see there was a patch with this issue earlier - will have to go through it after the final, hopefully will include in 3.6.1 - need to make sure nothing breaks.

Comment by Magnolia International [ 11/Sep/08 ]

Tom,

I'm finally about to apply your patch! Looks good so far, just one quick question: your modification of the isDefaultMapping() method doesn't seem to match the spec:

A string containing only the'/' character indicates the "default" servlet of the application. In this case the servlet path is the request URI minus the context path and the path info is null.

Am I missing something?

Comment by Magnolia International [ 11/Sep/08 ]

TODO : update task. In the default installation, we currently have 2 servlets mapped with /.magnolia/log4j* and /.magnolia/mail* respectively. It shouldn't be to complicated to write a task that would update other similar mappings.

note to self: a query like select * from mgnl:contentNode where jcr:path like '/server/filters/servlets/%/mappings/%' + a task along the lines of DoForAllQueryResult would help.

Comment by Magnolia International [ 11/Sep/08 ]

Tom: not applying your patch to SimpleUrlPattern.getEncodedString() - bits of untested magic in there, and after your changes, ServletDispatchingFilter doesn't use it anymore anyway

Comment by Magnolia International [ 11/Sep/08 ]

Patch applied on trunk, thanks a lot!
Will resolve this issue when

  • update task is written
  • I get your feedback regarding the default mapping
Comment by Magnolia International [ 16/Sep/08 ]

From Tom:

Yep, I missed implementing that part of the spec thanks for catching it Gregory!

(reply to jira mails doesn't do much, Tom, unless I accidentally see them )

Comment by Magnolia International [ 16/Sep/08 ]

fixed on 3.6 branch and trunk, update task added, specifically for our log4j and mail servlets which had non compliant mappings. Will document the change for custom servlets.

Generated at Mon Feb 12 03:33:33 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.