Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not honoring default value of jakarta.faces.FACELETS_BUFFER_SIZE (1024) #5164

Closed
volosied opened this issue Nov 4, 2022 · 4 comments
Closed

Comments

@volosied
Copy link
Contributor

volosied commented Nov 4, 2022

Describe the bug

The default buffer size is not set to 1024, as specified by the specification.

jakarta.faces.FACELETS_BUFFER_SIZE —The buffer size to set on the response when the ResponseWriter is generated. By default the value is 1024.

Tested with jakarta.faces-4.0.0.jar

To Reproduce

Steps to reproduce the behavior:
Test a page with:

<h:form>
    jakarta.faces.FACELETS_BUFFER_SIZE : #{externalContext.getResponse().getBufferSize()}
</h:form>

Expected behavior

1024 should be set.

Screenshots

If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: Mac
  • Browser FireFox

Additional context

Tomcat 10.1.0:
jakarta.faces.FACELETS_BUFFER_SIZE : 8192

Open Liberty (via facesContainer-4.0)
jakarta.faces.FACELETS_BUFFER_SIZE : 4096

Glassfish 7:
jakarta.faces.FACELETS_BUFFER_SIZE : 8192
(Interestingly, I had set FACELETS_BUFFER_SIZE to 1024 and 2048 here, but these values looks to be ignored...? Could there be a bug glassfish here...?)

@volosied
Copy link
Contributor Author

volosied commented Nov 4, 2022

https://github.com/eclipse-ee4j/mojarra/blob/master/impl/src/main/java/com/sun/faces/application/view/FaceletViewHandlingStrategy.java#L893-L896

It only calls setResponseBufferSize when the buffer size is set via web.xml. I think the if (responseBufferSizeSet) { should be removed.

        try {
            responseBufferSizeSet = webConfig.isSet(FaceletsBufferSize);
            responseBufferSize = Integer.parseInt(webConfig.getOptionValue(FaceletsBufferSize));
        } catch (NumberFormatException nfe) {
            responseBufferSize = Integer.parseInt(FaceletsBufferSize.getDefaultValue());
        }
        
... later on... 

        if (responseBufferSizeSet) {
            // set the buffer for content
            extContext.setResponseBufferSize(responseBufferSize);
        }

BalusC added a commit that referenced this issue Nov 12, 2022
remove check whether the response buffer size is set so that the default
value can properly be used
BalusC added a commit that referenced this issue Nov 12, 2022
while at it, noticed an unused context param for setting response buffer
size on JSP pages, took the opportunity to get rid of it altogether
@BalusC
Copy link
Contributor

BalusC commented Nov 12, 2022

Merged in 4.0 now.

@BalusC BalusC closed this as completed Nov 12, 2022
@BalusC
Copy link
Contributor

BalusC commented Nov 13, 2022

I ran TCK for #5171 and it turns out the following test was failing as regression of this change:

[INFO] Running ee.jakarta.tck.faces.test.servlet40.systemevent.Spec1135IT
Starting container using command: [/home/bauke/.sdkman/candidates/java/current/bin/java, -jar, /home/bauke/git/faces/tck/faces23/systemEvent/target/glassfish7/glassfish/modules/admin-cli.jar, start-domain, -t]
Successfully started the domain : domain1
domain  Location: /home/bauke/git/faces/tck/faces23/systemEvent/target/glassfish7/glassfish/domains/domain1
Log File: /home/bauke/git/faces/tck/faces23/systemEvent/target/glassfish7/glassfish/domains/domain1/logs/server.log
Admin Port: 4848
Nov 13, 2022 12:14:39 PM com.gargoylesoftware.htmlunit.WebClient printContentIfNecessary
INFO: statusCode=[500] contentType=[text/html]
Nov 13, 2022 12:14:39 PM com.gargoylesoftware.htmlunit.WebClient printContentIfNecessary
INFO: <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"><html xmlns="http://www.w3.org/1999/xhtml"><head><title>Eclipse GlassFish  7.0.0  - Error report</title><style type="text/css"><!--H1 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:22px;} H2 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:16px;} H3 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:14px;} BODY {font-family:Tahoma,Arial,sans-serif;color:black;background-color:white;} B {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;} P {font-family:Tahoma,Arial,sans-serif;background:white;color:black;font-size:12px;}A {color : black;}HR {color : #525D76;}--></style> </head><body><h1>HTTP Status 500 - Internal Server Error</h1><hr/><p><b>type</b> Exception report</p><p><b>message</b>Internal Server Error</p><p><b>description</b>The server encountered an internal error that prevented it from fulfilling this request.</p><p><b>exception</b> <pre>jakarta.servlet.ServletException: Cannot change buffer size after data has been written</pre></p><p><b>root cause</b> <pre>java.lang.IllegalStateException: Cannot change buffer size after data has been written</pre></p><p><b>note</b> <u>The full stack traces of the exception and its root causes are available in the Eclipse GlassFish  7.0.0  logs.</u></p><hr/><h3>Eclipse GlassFish  7.0.0 </h3></body></html>
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 10.868 s <<< FAILURE! - in ee.jakarta.tck.faces.test.servlet40.systemevent.Spec1135IT
[ERROR] ee.jakarta.tck.faces.test.servlet40.systemevent.Spec1135IT.testPreRenderViewEvent  Time elapsed: 0.961 s  <<< ERROR!
com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException: 500 Internal Server Error for http://localhost:8080/test-faces23-systemEvent/faces/postRenderViewEvent.xhtml
	at ee.jakarta.tck.faces.test.servlet40.systemevent.Spec1135IT.testPreRenderViewEvent(Spec1135IT.java:72)

BalusC added a commit to jakartaee/faces that referenced this issue Nov 13, 2022
eclipse-ee4j/mojarra#5164 -- it's basically
illegal to ExternalContext#getResponseOutputWriter() directly instead of
using FacesContext#getResponseWriter()
@BalusC
Copy link
Contributor

BalusC commented Nov 13, 2022

Test itself was illegal. I've fixed it.

arjantijms added a commit to jakartaee/faces that referenced this issue Nov 14, 2022
jasondlee pushed a commit to jboss/mojarra that referenced this issue May 18, 2023
remove check whether the response buffer size is set so that the default
value can properly be used
jasondlee pushed a commit to jboss/mojarra that referenced this issue May 18, 2023
while at it, noticed an unused context param for setting response buffer
size on JSP pages, took the opportunity to get rid of it altogether
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants