myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Marinschek <mmarinsc...@apache.org>
Subject Re: Fixing ResponseWriter.startCDATA/endCDATA
Date Fri, 23 Jul 2010 12:41:43 GMT
On Fri, Jul 23, 2010 at 2:40 PM, Matthias Wessendorf <matzew@apache.org> wrote:
> On Fri, Jul 23, 2010 at 2:30 PM, Bruno Aranda <brunoaranda@gmail.com> wrote:
>> Yeah Werner, you should take holidays seriously :)
>
> +1

ESPECIALLY this kind of holidays - a honeymoon ;)

best regards,

Martin

>> On 23 July 2010 13:28, Werner Punz <werner.punz@gmail.com> wrote:
>>>
>>> Hi I have written a load of tests for the PPR responsewriter to have it
>>> covered, but I will do some additional testing mid next week (and will
>>> commit those tests) if the ppr responsewriter still behaves as it should
>>> after the patch.
>>> Since we are not in the middle of another release I can guess the
>>> additional testing on the ppr responsewriter can wait a little bit.
>>> If anything negative comes out I probably can fix it on the ppr writers
>>> side.
>>>
>>>
>>>
>>> Werner
>>>
>>>
>>> Am 23.07.10 13:05, schrieb Matthias Wessendorf:
>>>>
>>>> re-tested; works fine with your patch as well
>>>>
>>>> On Fri, Jul 23, 2010 at 11:37 AM, Bruno Aranda<brunoaranda@gmail.com>
>>>>  wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> But the patch had not been checked it yet? Or did you patch it before
>>>>> trying
>>>>> the tests?
>>>>>
>>>>> In any case, I have committed it just now, all myfaces tests pass.
>>>>>
>>>>> Cheers!
>>>>>
>>>>> Bruno
>>>>>
>>>>> On 23 July 2010 08:26, Matthias Wessendorf<matzew@apache.org>  wrote:
>>>>>>
>>>>>> I just tested 2.0.2-SNAPSHOT with our internal version of ADF Faces,
>>>>>> that runs on JSF2.
>>>>>>
>>>>>> My jetty powered environment gives me no errors with our latest
>>>>>> trunk...
>>>>>>
>>>>>> -Matthias
>>>>>>
>>>>>> On Thu, Jul 22, 2010 at 9:36 PM, Werner Punz<werner.punz@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Btw. one issue about this, check if your fix does not break anything
>>>>>>> in
>>>>>>> the
>>>>>>> ppr case.
>>>>>>> The problem is that the ppr responseWriter simply delegates the
>>>>>>> HtmlResponseWriterImpl, but the issue is
>>>>>>> that CDATA blocks are automatically opened before any html is
>>>>>>> rendered,
>>>>>>> any
>>>>>>> valid CDATA block inside should not be swallowed but escaped
in that
>>>>>>> case.
>>>>>>>
>>>>>>>
>>>>>>> So a ppr response looks like this
>>>>>>>
>>>>>>> <changes>
>>>>>>>  <update id="bla"><![CDATA[<div id="bla">  ....</div>
 ]]></update>
>>>>>>>
>>>>>>> The problem is that per spec definition the xhr response must
be xml
>>>>>>> and any html must be embedded within CDATA blocks, now if there
is
>>>>>>> another
>>>>>>> CDATA block within the valid html it must be preserved at all
costs
>>>>>>> because
>>>>>>> it is vital that it is properly rendered at the ppr update time
(hence
>>>>>>> the
>>>>>>> complicated escaping in my code)
>>>>>>>
>>>>>>> Werner
>>>>>>>
>>>>>>>
>>>>>>> Am 22.07.10 21:31, schrieb Werner Punz:
>>>>>>>>
>>>>>>>> Ok point taken, yes the HTMLResponseWriterImpl definitely
is html so
>>>>>>>> a
>>>>>>>> fix there is valid.
>>>>>>>>
>>>>>>>>
>>>>>>>> Werner
>>>>>>>>
>>>>>>>>
>>>>>>>> Am 22.07.10 20:11, schrieb Bruno Aranda:
>>>>>>>>>
>>>>>>>>> But we are talking about the HtmlResponseWriterImpl,
which outputs
>>>>>>>>> HTML.
>>>>>>>>> The fix I have done is in that class and it only checks
if there is
>>>>>>>>> a
>>>>>>>>> CDATA already started before starting one when writing
the scripts.
>>>>>>>>> It
>>>>>>>>> is slightly different to the original problem (about
the
>>>>>>>>> HtmlResponse,
>>>>>>>>> which is different from the one in Mojarra). The fix
simply checks
>>>>>>>>> if
>>>>>>>>> there is the CDATA around the script before opening another
one
>>>>>>>>> inside
>>>>>>>>> the script. I think it is OK if we just do not nest CDATAs
in the
>>>>>>>>> HTML
>>>>>>>>> response, even if it was allowed.
>>>>>>>>>
>>>>>>>>> And this fixes the problems with PrimeFaces too, without
having to
>>>>>>>>> change the ResponseWriter or the PartialResponseWriterImpl...
>>>>>>>>>
>>>>>>>>> Bruno
>>>>>>>>>
>>>>>>>>> On 22 July 2010 16:59, Werner Punz<werner.punz@gmail.com
>>>>>>>>> <mailto:werner.punz@gmail.com>>  wrote:
>>>>>>>>>
>>>>>>>>> Hia guys please also read up on my jira response.
>>>>>>>>> The entire thing is not as easy as it seems, because
there are
>>>>>>>>> various ways a cdata block can be opened, first you can
do it via
>>>>>>>>> startCDATA secondly you can do it via a direct write.
>>>>>>>>>
>>>>>>>>> I did some kind of double buffering in case of a cdata
block was
>>>>>>>>> opened and then escaped the ]]>  as multiple cdata
blocks (the jira
>>>>>>>>> response explains the technique exactly)
>>>>>>>>>
>>>>>>>>> And yes there is somewhat a speed hit because of this,
but in case
>>>>>>>>> of the partial response writer I did not have a chance
because:
>>>>>>>>>
>>>>>>>>> But the partial response writer is somewhat different,
because it
>>>>>>>>> has to press html code in a valid xml response format,
so nested
>>>>>>>>> cdata blocks can occur naturally, the normal response
writer is
>>>>>>>>> somewhat different because nested cdata blocks are only
forbidden
>>>>>>>>> for xmlish output dialects others might allow it.
>>>>>>>>>
>>>>>>>>> Werner
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Am 22.07.10 17:47, schrieb Mark Struberg:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> But isn't the patch of Marcus Büttner doing this by
maintaining
>>>>>>>>> a reference
>>>>>>>>> counter?
>>>>>>>>>
>>>>>>>>> Another question: how is the performance of all this
>>>>>>>>> scanning/dynamic
>>>>>>>>> replacement?
>>>>>>>>>
>>>>>>>>> LieGrue,
>>>>>>>>> strub
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> From: Bruno Aranda<brunoaranda@gmail.com
>>>>>>>>> <mailto:brunoaranda@gmail.com>>
>>>>>>>>> To: MyFaces Development<dev@myfaces.apache.org
>>>>>>>>> <mailto:dev@myfaces.apache.org>>
>>>>>>>>> Sent: Thu, July 22, 2010 5:26:35 PM
>>>>>>>>> Subject: Re: Fixing ResponseWriter.startCDATA/endCDATA
>>>>>>>>>
>>>>>>>>> Further investigation of this incompatibility problem
with
>>>>>>>>> myfaces leads me to
>>>>>>>>> the fact that in the HtmlResponseWriterImpl, when we
write
>>>>>>>>> the content of a
>>>>>>>>> script, we create a CDATA element without checking if
is
>>>>>>>>> nested at all. That is
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> a problem, because if we use the standard response writer
>>>>>>>>> and we write a script
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> section inside a CDATA section, the problem will be triggered...
>>>>>>>>>
>>>>>>>>> We need a way in HtmlResponseWriterImpl to check nested
>>>>>>>>> CDATA calls to the
>>>>>>>>> startCDATA or endCDATA methods I guess.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>>
>>>>>>>>> Bruno
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 22 July 2010 15:15, Bruno Aranda<brunoaranda@gmail.com
>>>>>>>>> <mailto:brunoaranda@gmail.com>>  wrote:
>>>>>>>>>
>>>>>>>>> Just clicked on sent and Werner had answered in the JIRA
>>>>>>>>> issue explaining the
>>>>>>>>> partial approach...
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>>
>>>>>>>>> Bruno
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 22 July 2010 15:12, Bruno
>>>>>>>>> Aranda<brunoaranda@gmail.com
>>>>>>>>> <mailto:brunoaranda@gmail.com>>  wrote:
>>>>>>>>>
>>>>>>>>> As you can see in my black box tests with Mojarra, the
>>>>>>>>> behaviour is different in
>>>>>>>>>
>>>>>>>>> both implementations. In the base ResponseWriter class,
>>>>>>>>> they don't do anything
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> in the startCDATA method and throw an undocumented
>>>>>>>>> exception in the endCDATA.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> In both implementations of the base class, they
>>>>>>>>> throw an exception if the
>>>>>>>>> startCDATA method is called and it had been called
>>>>>>>>> already...
>>>>>>>>>
>>>>>>>>> I don't quite understand our implementation of the
>>>>>>>>> PartialResponseWriterImpl. We
>>>>>>>>>
>>>>>>>>> do buffer nested CDATAs and write them when closing
>>>>>>>>> the parent one? This would
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> still create nested CDATAs... I still need to
>>>>>>>>> understand this bit properly,
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>>
>>>>>>>>> Bruno
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 22 July 2010 13:58, Bruno
>>>>>>>>> Aranda<brunoaranda@gmail.com
>>>>>>>>> <mailto:brunoaranda@gmail.com>>  wrote:
>>>>>>>>>
>>>>>>>>> yeah, sorry, my problem was running only the API
>>>>>>>>> tests :)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Bruno
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 22 July 2010 13:48, Matthias
>>>>>>>>> Wessendorf<matzew@apache.org
>>>>>>>>> <mailto:matzew@apache.org>>  wrote:
>>>>>>>>>
>>>>>>>>> On Thu, Jul 22, 2010 at 2:14 PM, Matthias
>>>>>>>>> Wessendorf<matzew@apache.org
>>>>>>>>> <mailto:matzew@apache.org>>
>>>>>>>>>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> so, maybe there are now regressions?
>>>>>>>>>
>>>>>>>>> hrm. have you done some testing?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Ah, the discussion is on the JIRA..
>>>>>>>>>
>>>>>>>>> please run tests, before committing ;-)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -M
>>>>>>>>>
>>>>>>>>> On Thu, Jul 22, 2010 at 2:07 PM,
>>>>>>>>> Matthias Wessendorf<matzew@apache.org
>>>>>>>>> <mailto:matzew@apache.org>>
>>>>>>>>>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> sounds right.
>>>>>>>>>
>>>>>>>>> does blame say more why it does not
>>>>>>>>> do nothing?
>>>>>>>>>
>>>>>>>>> It is also kinda strange since the
>>>>>>>>> TCK was successfully executed for
>>>>>>>>> 2.0.0 and 2.0.1;
>>>>>>>>>
>>>>>>>>> -Matthias
>>>>>>>>>
>>>>>>>>> On Thu, Jul 22, 2010 at 1:48 PM,
>>>>>>>>> Bruno Aranda<brunoaranda@gmail.com
>>>>>>>>> <mailto:brunoaranda@gmail.com>>
>>>>>>>>>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Having problems with Primefaces
>>>>>>>>> again I have realised that something
>>>>>>>>>
>>>>>>>>> was
>>>>>>>>>
>>>>>>>>> working with Mojarra, but not
>>>>>>>>> with MyFaces. Again, is the
>>>>>>>>> ResponseWriter.startCDATA stuff
>>>>>>>>> which Primefaces invokes directly on
>>>>>>>>>
>>>>>>>>> its
>>>>>>>>>
>>>>>>>>> main phase listener.
>>>>>>>>>
>>>>>>>>> However, reading the javadocs:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> https://javaserverfaces.dev.java.net/nonav/docs/2.0/javadocs/index.html
>>>>>>>>>
>>>>>>>>> It says that method "should
>>>>>>>>> take no action when invoked"...
>>>>>>>>> which
>>>>>>>>>
>>>>>>>>> means
>>>>>>>>>
>>>>>>>>> that it should be completely
>>>>>>>>> empty as far as I understand. If
>>>>>>>>> that was
>>>>>>>>>
>>>>>>>>> the
>>>>>>>>>
>>>>>>>>> case, we would get the same
>>>>>>>>> behaviour in both implementations...
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>>
>>>>>>>>> Bruno
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Matthias Wessendorf
>>>>>>>>>
>>>>>>>>> blog:
>>>>>>>>> http://matthiaswessendorf.wordpress.com/
>>>>>>>>> sessions:
>>>>>>>>> http://www.slideshare.net/mwessendorf
>>>>>>>>> twitter: http://twitter.com/mwessendorf
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Matthias Wessendorf
>>>>>>>>>
>>>>>>>>> blog:
>>>>>>>>> http://matthiaswessendorf.wordpress.com/
>>>>>>>>> sessions:
>>>>>>>>> http://www.slideshare.net/mwessendorf
>>>>>>>>> twitter: http://twitter.com/mwessendorf
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>>
>>>>>>>>> Matthias Wessendorf
>>>>>>>>>
>>>>>>>>> blog: http://matthiaswessendorf.wordpress.com/
>>>>>>>>> sessions: http://www.slideshare.net/mwessendorf
>>>>>>>>> twitter: http://twitter.com/mwessendorf
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Matthias Wessendorf
>>>>>>
>>>>>> blog: http://matthiaswessendorf.wordpress.com/
>>>>>> sessions: http://www.slideshare.net/mwessendorf
>>>>>> twitter: http://twitter.com/mwessendorf
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
>
> --
> Matthias Wessendorf
>
> blog: http://matthiaswessendorf.wordpress.com/
> sessions: http://www.slideshare.net/mwessendorf
> twitter: http://twitter.com/mwessendorf
>



-- 

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces

Mime
View raw message