jakartaee / jakartaee/servlet

Clarify methods under various dispatch types.

Open
#309 15 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
325
Forks
112
PR merge metrics
No merged PRs in 30d

Description

While considering #308 and some jetty TCK failures with getHttpServletMapping, it became apparent that there are some inconsistent and undefined behaviours under various dispatch types.
In the summary below I have indicated if under a given dispatch type if a method should return values for:
- **target** The target of the current dispatch, ie the Servlet that a dispatch is to.
- **source** The source of the current dispatch, ie the Servlet that was the target when the dispatch was done or when the async cycle was started.
- **target^source** Merged values of both the source and target, with target having precedence.
- **target||source** Target value if there is one, else the source value.

  | REQUEST | FORWARD | NAMED FORWARD | INCLUDE | NAMED INCLUDE | ASYNC | ERROR
-- | -- | -- | -- | -- | -- | -- | --
getRequetURI | target | target | source[9] | source | source[9] | target | target
get(Context\|Servlet)Path(Info)? | target | target | source[9] | source | source[9] | target | target
getParameter | target | target^source | source[9] | target^source | source[9] | target^source | target[7]
getQueryString | target | target\|\|source[1] | source[9] | source | source[9] | target\|\|source | target[8]
getHttpServletMapping | target | target | source[9] | source | source[9] | source[2] | target
isUserInRole(String) | target | target | source[9] | target[4] | source[9] | target | target
getAttribute("javax.servlet.forward.*") |   | source |   |   |  | |
getAttribute("javax.servlet.include.*") |   |   | | target |   |  |
getAttribute("javax.servlet.async.*) |   |   | | |  | source |  
getAttribute("javax.servlet.async.mapping") |   |   |  | | | source[3] |  
sendError(); getAttribute("javax.servlet.error.*") | target | target | source[9] | target[5] | source[9] | target | target
getSession | target | target | source[9] | target | source[9] | target | target
getPathTranslated | target | target | source[9] | target[6] | source[9] | target | target

Mostly, (other than the bizarre behaviour of include that I never understood and probably lost in ancient history) this table looks pretty sensible. However there are a number of cases that I think need to be clarified:

**[1]** As per the discussion in #308, the spec doesn't really say what the query string should be during a `FORWARD`, only what the parameters should be. Jetty currently merges the query string of source and target, while tomcat and undertow use the target if there is one else the source.

**[2]** Unlike the path methods which return the target values during an `ASYNC` dispatch, the getHttpServletMapping method is [explicitly documented](https://javadoc.io/static/javax.servlet/javax.servlet-api/4.0.1/javax/servlet/http/HttpServletRequest.html#getHttpServletMapping--) as returning the source mapping (actually I'm a bit dubious about the language in that javadoc, but this interpretation is backed up by a [TCK test](https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/servlet/api/javax_servlet_http/httpservletrequest40/Client.java#L210-L214)). Is this really the intended behaviour? Why is it just this method that acts like an `INCLUDE` during `ASYNC`?

**[3]** Given the current interpretation of [2], this means that the attribute `javax.servlet.async.mapping` also [returns the source mapping](https://javadoc.io/doc/javax.servlet/javax.servlet-api/latest/javax/servlet/RequestDispatcher.html#FORWARD_MAPPING) and thus the target mapping is unavailable during an `ASYNC` dispatch. Should this attribute be set to the target value?

**[4]** I can't find anything to say if `isUserInRole` should be affected by the target Servlet role mappings during an include. During an include, almost all other methods return values of the source, so perhaps the also the sources role mappings should apply? But that is also kind of counter intuitive as a included resource might need it's role mappings and runAs role to generate its content? Perhaps the role mappings should be merged???

**[5]** If `sendError` is called during an `INCLUDE`, it is unclear what the `javax.servlet.error.*` attributes should be during any subsequent `ERROR` dispatch. It feels reasonable that it should be the target... but then I could say that for all methods during an include?

**[6]** If `getPathTranslated` is called during an `INCLUDE`, it is unclear what the path should be relative to the source or target context. It feels reasonable that it should be the target... but then I could say that for all methods during an include?

**[7]** Probably should be `target^source`

**[8]** Maybe should be `target||source`

**[9]** Named dispatchers are not well defined

Edit: reworded [4]
Edit: added [7]
Edit: added [8]
Edit: added [9]

Contributor guide

Open the contributing guide

Research direction

Start with the dispatch-behavior table in this issue, discussion #308, and the cited getHttpServletMapping and RequestDispatcher documentation. Review the referenced Jakarta TCK Client.java test to compare the documented and tested behavior. Done means the ambiguities have an agreed interpretation and the relevant specification, Javadocs, or tests reflect it.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api, backend
Issue type
Documentation
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.