search

jakartaee / faces

Jakarta Faces
Other
102 stars 55 forks source link

TCK Challenge: faces22/cdiMethodValidation uses inadequate buffer size on Response Object #1716

Closed brideck closed 1 year ago

brideck commented 1 year ago

Challenged Tests: ee.jakarta.tck.faces.test.javaee7.cdimethodvalidation.cdimethodvalidation.MethodValidationIT#testIncorrectUsage

TCK Version: Jakarta Faces 4.0.x

Tested Implementation: Open Liberty -- containing MyFaces 4.0

Description: Section 11.1.3 of the Faces 4.0 specification states that the default value for FACELETS_BUFFER_SIZE is 1024. Unfortunately, the TCK application contains a lot of text, and the default value is not large enough for it.

In Open Liberty, the test fails because the response is committed when the buffer size is reached, doing so with a 200 status. Processing continues and the expected ConstraintViolationException occurs, which is appended to the response, but the test fails because the expected status of 500 is not received.

The solution here would be to update the FACELETS_BUFFER_SIZE in the application to a higher value, like 4096.

This is not a problem for implementations using Mojarra because it does not appear to honor the spec-defined default value. See https://github.com/eclipse-ee4j/mojarra/issues/5164 for details.

pnicolucci commented 1 year ago

This is valid from my perspective. @BalusC @arjantijms what do you think?

BalusC commented 1 year ago

Hmm this test still passes for Mojarra 4.0.1-SNAPSHOT including the fix for https://github.com/eclipse-ee4j/mojarra/issues/5164

volosied commented 1 year ago

Hi, @BalusC

I tested this and I think Glassfish has a bug somewhere. I added jakarta.faces.FACELETS_BUFFER_SIZE : #{externalContext.getResponse().getBufferSize()} to the index.xhtml page to verify the buffer size and I see 8192 still set.

image

Tested on GlassFish 7.0.0-M4 and Mojarra 4.1.0-SNAPSHOT (latest code on main)

If this test ran on Tomcat or Jetty, I would expect it to fail there due the 5164 change.

volosied commented 1 year ago

@arjantijms I encountered this buffer size problem again after fixing MyFaces to match the expected behavior in https://github.com/jakartaee/faces/blob/e6f0e26e4c5ee39d192505a3c8bc0e41d8c05d2d/tck/faces22/multiFieldValidation/src/test/java/com/sun/faces/test/javaee7/multiFieldValidation/Spec1IT.java#L181

Should we have another challenged opened for that test or can this buffer size issue be followed up here?