dev@javaserverfaces.java.net

Re: [REVIEW/629] jsf 1.2 and <jsp:param> not working

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Tue, 02 Oct 2007 08:17:28 -0700

Ryan Lubke wrote:
> Ryan Lubke wrote:
>> Adam Winer wrote:
>>> Ryan Lubke said the following on 9/21/07 12:03 AM PT:
>>>> Adam Winer wrote:
>>>>> I'm frankly amazed at the claim this worked in 1.1.
>>>>> The EL in 1.1 was bound to ExternalContext.getRequestParameterMap(),
>>>>> which is always the ServletRequest parameter map, *not* the
>>>>> current JSP parameter map - or was there magic in 1.1 that I didn't
>>>>> know about?
>>>> Not following you here Adam. What JSP parameteter map?
>>>> When a JSP include is performed and it specifies parameters, those
>>>> params will be present for the life of the include via the
>>>> ServletRequest.
>>>
>>> Hrm - I always guessed that it was a different ServletRequest
>>> instance that gets created for the include to add the needed
>>> parameters, instead of the servlet engine hacking into
>>> its own object to add and then later remove the parameters.
>>> (It's how I'd implement a servlet engine, at any rate.)
>>> Does the servlet spec really require it's the same instance
>>> across includes?
>> Ah, ok. By default, the container *does* wrap the request (I added
>> println() calls in each JSP to confirm.
>>
>> I made the assumption that the request had to be the same in order for
>> this to have ever worked in 1.1. So how did it work then? Running the
>> same set of modified JSPs in 1.1 show:
>>
>> 05 -> com.sun.faces.context.MyHttpServletRequestWrapper_at_1ae90c
>> inc1 -> com.sun.faces.context.MyHttpServletRequestWrapper_at_1ae90c
>> inc2 -> com.sun.faces.context.MyHttpServletRequestWrapper_at_1ae90c
>> inc3 -> com.sun.faces.context.MyHttpServletRequestWrapper_at_1ae90c
>> inc2 -> com.sun.faces.context.MyHttpServletRequestWrapper_at_1ae90c
>> inc3 -> com.sun.faces.context.MyHttpServletRequestWrapper_at_1ae90c
>>
>> This was present in the ctor of ExternalContextImpl in 1.1:
>>
>> if (RIConstants.IS_UNIT_TEST_MODE) {
>> this.request = request;
>> } else {
>> // PENDING(craigmcc) - Work around a Tomcat 4.1 and 5.0 bug
>> // where the request wrapper used on a
>> // RequestDispatcher.forward() call delegates
>> // removeAttribute() and setAttribute() to the wrapped
>> // request, but not getAttribute(). This causes attributes
>> // set via the RequestMap returned in this class to not be
>> // visible via calls to getAttribute() on the underlying
>> // request.
>> if (request instanceof HttpServletRequest) {
>> this.request = new MyHttpServletRequestWrapper
>> ((HttpServletRequest) request);
>> } else {
>> this.request = new MyServletRequestWrapper(request);
>> }
>> }
>>
>> So technically, this shouldn't have worked as you stated earlier.
>>
> At this point, I think the best course of action is not to fix this:
> - the spec doesn't require this support
> - it only worked as a side effect of another fix and as such
> wouldn't have worked if that container didn't have an issue
> - the fix may introduce some snags that we don't want to have
> to deal with down the road.
>
> Thoughts? Comments?
Assuming silence implies consent, so I'm going to close.

>
>
>>>
>>>> Because of that, and how we build/rendered the tree in 1.1, it works.
>>>
>>> If and only if:
>>> - It's guaranteed to be the same ServletRequest instance
>>> - You didn't have a rendersChildren parent in the parent page
>>> around the include
>>>
>>> -- Adam
>>>
>>>>
>>>>>
>>>>> -- Adam
>>>>>
>>>>>
>>>>> Ryan Lubke wrote:
>>>>>> Change bundle attached to the issue [1].
>>>>>> Please note I've added additional comments to the issue for
>>>>>> consideration
>>>>>> after having had attached the change bundle.
>>>>>>
>>>>>>
>>>>>> [1] https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=629
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>>
>>>>>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>>>>>> For additional commands, e-mail:
>>>>>> dev-help_at_javaserverfaces.dev.java.net
>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>>>>> For additional commands, e-mail:
>>>>> dev-help_at_javaserverfaces.dev.java.net
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>>>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>