[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: |
|
||||||||||||||||
| Issue Links: |
|
||||||||||||||||
| 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. 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:
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!
|
| Comment by Magnolia International [ 16/Sep/08 ] |
|
From Tom:
(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. |