eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
158 stars 107 forks source link

Duplicate `<changes>` entry in PrimeFaces PartialResponseWriter #5456

Closed melloware closed 1 day ago

melloware commented 5 days ago

Describe the bug

Original Report: https://github.com/primefaces/primefaces/issues/12189

in PrimeFaces 14.0.0 we made this change per @BalusC recommendation: https://github.com/primefaces/primefaces/issues/9518

When using ajax="true" and primefaces.CSP enabled the FileDownload is executed twice. I have debugged it and it seems to be Mojarra is writing the <changes> entry twice and MyFaces is only writing it once.

Mojarra 4.0.7

<?xml version='1.0' encoding='UTF-8'?>
<partial-response>
    <changes>
        <update id="j_id1:jakarta.faces.ViewState:0"><![CDATA[1583319729054827418:-7566133317078344421]]></update>
        <eval><![CDATA[if(window.PrimeFaces){PrimeFaces.csp.init('MjAzM2FlNWUtMzk3Ny00OWRlLWJjMGMtYjBkZmYwOTI2YmU4');};;PrimeFaces.download('/jakarta.faces.resource/dynamiccontent.properties.xhtml?ln=primefaces&v=14.0.1&pfdrid=c07524ce314cee24b9418afbab32a60f&pfdrt=sc&pfdrid_c=false&uid=2dc9d154-ce3e-4490-951b-6c624acfde8e', 'text/plain', 'default.txt', 'primefaces.download_test');]]></eval>
    </changes>
    <changes>
        <eval><![CDATA[if(window.PrimeFaces){PrimeFaces.csp.init('MjAzM2FlNWUtMzk3Ny00OWRlLWJjMGMtYjBkZmYwOTI2YmU4');};;PrimeFaces.download('/jakarta.faces.resource/dynamiccontent.properties.xhtml?ln=primefaces&v=14.0.1&pfdrid=c07524ce314cee24b9418afbab32a60f&pfdrt=sc&pfdrid_c=false&uid=2dc9d154-ce3e-4490-951b-6c624acfde8e', 'text/plain', 'default.txt', 'primefaces.download_test');]]></eval>
    </changes>
</partial-response>

MyFaces 4.0.2

<?xml version="1.0" encoding="UTF-8"?>
<partial-response id="j_id__v_0">
    <changes>
        <update id="jakarta.faces.Resource"><![CDATA[]]></update>
        <update id="j_id__v_0:jakarta.faces.ViewState:1"><![CDATA[N2RiOTM1NWQ5ZDA5YjJjODAwMDAwMDAx]]></update>
        <eval><![CDATA[if(window.PrimeFaces){PrimeFaces.csp.init('MmE5MTA0ZWQtNTJkMC00OTExLTkyYmQtOTkyYjNhYWY1NmRk');};;PrimeFaces.download('/jakarta.faces.resource/dynamiccontent.properties.xhtml?ln=primefaces&v=14.0.1&pfdrid=c07524ce314cee24b9418afbab32a60f&pfdrt=sc&pfdrid_c=false&uid=1a422c9b-b912-4c60-a18b-0523361238f6', 'text/plain', 'default.txt', 'primefaces.download_test');]]></eval>
    </changes>
</partial-response>

Reproducer

pf-12189.zip

  1. Open the Dialog and press Download or Download AJAX.
  2. With non-ajax download it works fine.
  3. With AJAX download you will see two duplicate files download default.txt and default(1).txt.

Expected behavior

Only 1 file downloaded.

PrimeFaces edition

Community

PrimeFaces version

14.0.1

Theme

No response

JSF implementation

Mojarra

JSF version

2.3

Java version

11

melloware commented 5 days ago

Found a workaround but can't explain why...

https://github.com/primefaces/primefaces/pull/12192

BalusC commented 1 day ago

It's because CspPartialResponseWriter extends PrimePartialResponseWriter so PrimePartialResponseWriter#endDocument() impl, which adds eval scripts, is invoked twice. Make it extend PartialResponseWriter instead and the problem will go away.

melloware commented 1 day ago

let me test that. thank you.

melloware commented 1 day ago

OK so that fixes Mojarra and break MyFaces. The new result is this...

Mojarra

<?xml version='1.0' encoding='utf-8'?>
<partial-response>
    <changes>
        <update id="j_id1:javax.faces.ViewState:0"><![CDATA[-8888652108271766523:-9198939220272553376]]></update>
    </changes>
    <changes>
        <eval><![CDATA[if(window.PrimeFaces){PrimeFaces.csp.init('N2QyNTFlYWYtOGM5MS00NjBkLWFkN2EtODExOWVlMDkyMDU4');};;PrimeFaces.download('/javax.faces.resource/dynamiccontent.properties.xhtml?ln=primefaces&v=15.0.0-SNAPSHOT&pfdrid=c07524ce314cee24b9418afbab32a60f&pfdrt=sc&pfdrid_c=false&uid=b4ffba85-2f08-4766-9001-f3fed0f52ba1', 'text/plain', 'default.txt', 'primefaces.download_test');]]></eval>
    </changes>
</partial-response>

MyFaces (missing eval script)

<?xml version="1.0" encoding="UTF-8"?>
<partial-response id="j_id__v_0">
    <changes>
        <update id="javax.faces.Resource"><![CDATA[]]></update>
        <update id="j_id__v_0:javax.faces.ViewState:1"><![CDATA[ODM4NzY3MzRCMTc2RUY4RDAwMDAwMDAx]]></update>
    </changes>
</partial-response>
melloware commented 1 day ago

OK with your help got to the bottom of it the real problem was in PrimePartialViewContent passing the writer into a writer.

https://github.com/primefaces/primefaces/pull/12192

BalusC commented 1 day ago

Hmm ok probably MyFaces still has a wrapped-vs-getWrapped bug in its ResponseWriter(Wrapper) impl that it fails to invoke the PrimePartialResponseWriter#endDocument when given a wrapped writer.

melloware commented 1 day ago

Agreed I will take a look at MyFaces