myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Werner Punz <werner.p...@gmail.com>
Subject Re: Fixing ResponseWriter.startCDATA/endCDATA
Date Fri, 23 Jul 2010 12:48:17 GMT
The honeymoon is over already, unfortunately, since we have a small boy 
of 19 months we only could get away for four days to one of the local lakes.

Werner


Am 23.07.10 14:41, schrieb Martin Marinschek:
> 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
>>
>
>
>



Mime
View raw message