Re: Fixing ResponseWriter.startCDATA/endCDATA

Fri, 23 Jul 2010 12:03:03 -0700

Mark you probably have been to often to the Burgenland, there are lakes in Austria which are not flooded with those dreaded insects. I really can recommend one of the mountain lakes where we have been. 
No Gelsen there just Pferdebremsen :-D


Am 23.07.10 15:05, schrieb Mark Struberg:

that means you are now awarded not only with 'Master of JavaScript' but also
with the title 'Gelsinator' :)

LieGrue,
strub



----- Original Message ----

From: Werner Punz<[email protected]>
To: [email protected]
Sent: Fri, July 23, 2010 2:48:17 PM
Subject: Re: Fixing ResponseWriter.startCDATA/endCDATA

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<[email protected]>

wrote:

On Fri, Jul 23, 2010 at 2:30 PM, Bruno Aranda<[email protected]>

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<[email protected]>    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<[email protected]>
    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<[email protected]>

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<[email protected]>
  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<[email protected]
  <mailto:[email protected]>>       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<[email protected]
  <mailto:[email protected]>>
  To: MyFaces Development<[email protected]
  <mailto:[email protected]>>
  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<[email protected]
  <mailto:[email protected]>>       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<[email protected]
  <mailto:[email protected]>>       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<[email protected]
  <mailto:[email protected]>>       wrote:

  yeah, sorry, my problem was running only the  API
tests  :)


Bruno



; On 22 July 2010 13:48, Matthias
  Wessendorf<[email protected]
  <mailto:[email protected]>>      wrote:

  On Thu, Jul 22, 2010 at 2:14 PM,  Matthias
Wessendorf<[email protected]
  <mailto:[email protected]>>

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<[email protected]
  <mailto:[email protected]>>

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<[email protected]
  <mailto:[email protected]>>

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

Reply via email to