jakartaee / platform-tck

Jakartaee-tck
Other
124 stars 103 forks source link

Old DTDs/Schema in deployment descriptors of Tags/Pages/Servlet/Assembly Platform TCK tests #1313

Open gurunrao opened 1 month ago

gurunrao commented 1 month ago

Effected Jakarta EE Platform TCK version 9.1/10.0.

Platform specification 9.1/10 states "support for versions of DTDs and schemas prior to Jakarta EE 8 is optional". Platform specification detailed reference on support of previous schema can be found at: https://jakarta.ee/specifications/platform/10/jakarta-platform-spec-10.0#a3447

Following tests have old schema[ http://java.sun.com/dtd/web-app_2_3.dtd ] reference in the web.xml descriptor.

Tags tests:
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#negativeScriptFreeTlvNoDeclTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveBundleLocalizationScopeTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveCatchVarTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveCWOTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveDateParamQueryTimestampTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveDefaultEncodingTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveFDValueTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveFNScopeTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveFormatLocalizationContextI18NTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveForTokensTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveIfScopeTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveImportAbsUrlTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveItemsObjArrayTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveJSTLURITest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveMessageKeyTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveOutEscXmlTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveParamNameValueTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveParamValueTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveParseVarTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positivePDValueTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positivePermittedTlvTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positivePNValueTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveQueryScopeAttributeTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveRedirectTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveRemoveScopeVarTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveResultGetRowsCountTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetBundleScopeVarTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetDataSourceScopeAttributeTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetLocaleValueTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetScopeTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetSelectVarTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetTimezoneValueTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveTimezoneValueTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveTransformVarTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveTxQueryCommitTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveUpdateBodyContentTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveUrlScopeTest
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveXParamNameTest

Pages tests:
com/sun/ts/tests/jsp/spec/configuration/elevaluation/URLClient.java#elEvaluation23WebApplicationTest

Following servlet test have old schema [ http://java.sun.com/j2ee/dtds/web-app_2_2.dtd ] reference in the web.xml descriptor.

com/sun/ts/tests/servlet/compat/LeadingSlash/WithLeadingSlash/URLClient.java#WithLeadingSlashTest
com/sun/ts/tests/servlet/compat/LeadingSlash/WithoutLeadingSlash/URLClient.java#WithoutLeadingSlashTest

Following assembly test have old schema [ http://java.sun.com/j2ee/dtds/web-app_2_2.dtd ] reference in the web.xml descriptor.

  com/sun/ts/tests/assembly/compat/standalone/war/compat12_13/Client.java#testStandaloneWar
  com/sun/ts/tests/assembly/compat/standalone/war/compat12_14/Client.java#testStandaloneWar
  com/sun/ts/tests/assembly/compat/standalone/war/compat12_50/Client.java#testStandaloneWar
  com/sun/ts/tests/assembly/compat/standalone/war/compat13_14/Client.java#testStandaloneWar

Above mentioned tests violate Platform specification optional support for versions of DTDs/schemas prior to Jakarta EE 8 and make support of older DTDs/schema mandatory for the vendors of Jakarta EE 9.1/10. The Tests are failing with deployment failures of test war/ear in Application servers which do not support old DTDs/schema.

scottmarlow commented 1 month ago

It sounds like this is a Platform TCK challenge for the mentioned tests so the Platform Spec team should approve or deny the challenge.

Note that there is some overlap with the https://github.com/jakartaee/tags/issues/255 challenge:

com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveFormatLocalizationContextI18NTest com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveTimezoneValueTest

CC @pnicolucci @markt-asf

scottmarlow commented 1 month ago

Email sent to Platform to vote on this challenge as well as BCC sent to JSTL, JSP, Servlet mailing lists https://www.eclipse.org/lists/jakartaee-platform-dev/msg04461.html

ivargrimstad commented 1 month ago

+1

gurunrao commented 1 month ago

@scottmarlow - added 4 more tests from assembly module of platform TCK.

pnicolucci commented 4 weeks ago

@scottmarlow from a Jakarta Tags perspective are you asking to update the schema version in the deployment descriptors for the application?

scottmarlow commented 4 weeks ago

@scottmarlow from a Jakarta Tags perspective are you asking to update the schema version in the deployment descriptors for the application?

@pnicolucci If this TCK challenge is accepted, we would either exclude the mentioned tests or update the schema version to something like:

http://java.sun.com/dtd/web-app_2_3.dtd + http://java.sun.com/j2ee/dtds/web-app_2_2.dtd to https://jakarta.ee/xml/ns/jakartaee/web-app_5_0.xsd

http://www.sun.com/software/sunone/appserver/dtds/sun-application-client_1_4-0.dtd + http://www.sun.com/software/sunone/appserver/dtds/sun-application-client_5_0-0.dtd to https://jakarta.ee/xml/ns/jakartaee/application-client_10.xsd

@gurunrao do you agree?

Email sent to Platform to vote on this challenge as well as BCC sent to JSTL, JSP, Servlet mailing lists https://www.eclipse.org/lists/jakartaee-platform-dev/msg04461.html

FYI, the email wasn't sent to JSTL, JSP, Servlet mailing list as but it did go to the Platform mailing list.

gurunrao commented 4 weeks ago

@scottmarlow from a Jakarta Tags perspective are you asking to update the schema version in the deployment descriptors for the application?

@pnicolucci If this TCK challenge is accepted, we would either exclude the mentioned tests or update the schema version to something like:

http://java.sun.com/dtd/web-app_2_3.dtd + http://java.sun.com/j2ee/dtds/web-app_2_2.dtd to https://jakarta.ee/xml/ns/jakartaee/web-app_5_0.xsd

http://www.sun.com/software/sunone/appserver/dtds/sun-application-client_1_4-0.dtd + http://www.sun.com/software/sunone/appserver/dtds/sun-application-client_5_0-0.dtd to https://jakarta.ee/xml/ns/jakartaee/application-client_10.xsd

@gurunrao do you agree?

Email sent to Platform to vote on this challenge as well as BCC sent to JSTL, JSP, Servlet mailing lists https://www.eclipse.org/lists/jakartaee-platform-dev/msg04461.html

FYI, the email wasn't sent to JSTL, JSP, Servlet mailing list as but it did go to the Platform mailing list.

we should update schema to Jakarta EE 8+ schema, as per Platform Specification document. Document reference can be found at https://jakarta.ee/specifications/platform/10/jakarta-platform-spec-10.0#a3447

scottmarlow commented 4 weeks ago

we should update schema to Jakarta EE 8+ schema, as per Platform Specification document. Document reference can be found at https://jakarta.ee/specifications/platform/10/jakarta-platform-spec-10.0#a3447

If we update to EE 9+ schemas, that would be helpful for the Tags TCK testing with Jakarta EE 11.

scottmarlow commented 3 weeks ago

As mentioned in the Platform TCK call, I think this challenge is likely to be approved.

We mentioned in the call that this challenge is for the EE 9.1 + 10.0 Platform TCK but the related component specs (Tags/Pages/Servlet) could opt into having component TCKs released as well to address this challenge.

CC @pnicolucci @markt-asf

scottmarlow commented 2 weeks ago

My vote is +1 for this change which means there are two votes for and there no votes against. I think that with only two votes this is a discussion but not a valid vote. I'll read up more on this in the Eclipse handbook.

I am voting for this challenge as it was pretty well discussed during the EE 9 development cycle and is explained in the Platform Spec:

Appendix A: Previous Version Deployment Descriptors This appendix describes Document Type Definitions (DTDs) and XML schemas for Deployment Descriptors from previous versions of the Java™ EE and Jakarta™ EE specifications. All Jakarta EE 9 products are required to support the DTDs and schemas specified by Jakarta EE 8, as well as the schemas specified in this version of the specification. This ensures that applications written to the previous version of the Jakarta EE specification can be deployed on products supporting the current version of this specification. Support for versions of DTDs and schemas prior to Jakarta EE 8 is optional. In addition, there are no restrictions on mixing versions of supported deployment descriptors in a single application; any combination of valid deployment descriptor versions must be supported.

scottmarlow commented 2 weeks ago

Echoing from https://www.eclipse.org/projects/handbook/#4_7_Committers_and_Contributors:

Committers are required to monitor the mailing lists associated with the Project. This is a condition of being granted commit rights to the Project. It is mandatory because Committers must participate in votes (which in some cases require a certain minimum number of votes) and must respond to the mailing list in a timely fashion in order to facilitate the smooth operation of the Project. When a Committer is granted commit rights they will be added to the appropriate mailing lists. A Committer must not be unsubscribed from a Developer mailing list unless their associated commit privileges are also revoked.

Committers are required to track, participate in, and vote on, relevant discussions in their associated Projects. There are three voting responses: +1 (yes), -1 (no, or veto), and 0 (abstain).

^ explains that voting should happen on the mailing list. We should try that for the next TCK challenge that needs input from the Platform committers.

I'm going to accept this challenge as per the concrete information in https://jakarta.ee/specifications/platform/10/jakarta-platform-spec-10.0#a3447 since no vote should be needed.

pnicolucci commented 1 week ago

@scottmarlow what's the plan for the EE11 TCK for these tests? We've done a lot of work so far for JSTL to be able to run the same tck for EE10 and EE11 (that's my understanding anyhow). It would be nice if we could update the deployment descriptors rather than exclude the tests. Is there something I can do from the JSTL perspective so the tests are not excluded?

scottmarlow commented 1 week ago

@scottmarlow what's the plan for the EE11 TCK for these tests?

I created issue https://github.com/jakartaee/platform-tck/issues/1332 for EE 11 to update all tests to use the latest EE schemas which does include the Tags TCK tests.

We've done a lot of work so far for JSTL to be able to run the same tck for EE10 and EE11 (that's my understanding anyhow). It would be nice if we could update the deployment descriptors rather than exclude the tests. Is there something I can do from the JSTL perspective so the tests are not excluded?

Thanks @pnicolucci for asking!

Locally on the 10.0.x branch, I see the following:

grep -rli "http://java.sun.com/dtd/web-app_2_3.dtd"

src/com/sun/ts/tests/assembly/compat/standalone/war/compat12_13/assembly_compat_standalone_war_compat12_13_component_jsp_web.xml src/com/sun/ts/tests/assembly/compat/standalone/war/compat13_14/WEB-INF/web.xml src/com/sun/ts/tests/connector/localTx/transinflow/transinflow_jsp_vehicle_web.xml src/com/sun/ts/tests/connector/localTx/transinflow/transinflow_servlet_vehicle_servlet.xml src/com/sun/ts/tests/connector/localTx/workcontext/workcontext_jsp_vehicle_web.xml src/com/sun/ts/tests/connector/localTx/workcontext/workcontext_servlet_vehicle_servlet.xml src/com/sun/ts/tests/connector/noTx/lifecycle/lifecycle_jsp_vehicle_web.xml src/com/sun/ts/tests/connector/noTx/lifecycle/lifecycle_servlet_vehicle_servlet.xml src/com/sun/ts/tests/connector/noTx/workmgt/workmgt_jsp_vehicle_web.xml src/com/sun/ts/tests/connector/noTx/workmgt/workmgt_servlet_vehicle_servlet.xml src/com/sun/ts/tests/connector/xa/lifecycle/xa_lifecycle_jsp_vehicle_web.xml src/com/sun/ts/tests/connector/xa/lifecycle/xa_lifecycle_servlet_vehicle_servlet.xml src/com/sun/ts/tests/connector/xa/workmgt/xa_workmgt_jsp_vehicle_web.xml src/com/sun/ts/tests/connector/xa/workmgt/xa_workmgt_servlet_vehicle_servlet.xml src/com/sun/ts/tests/jsp/spec/configuration/elevaluation/jsp_config_eleval23_web.xml src/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.xml

An alternative could be to update ^ to https://jakarta.ee/xml/ns/jakartaee/web-app_6_0.xsd for EE 10 which in the case of the Tags component (standalone) TCK would help with EE 11 as well.

@gurunrao what do you think about further test changes to update the ^ schema references for EE 10 as an alternative?

scottmarlow commented 1 week ago

Also created https://github.com/jakartaee/platform/issues/906 to clarify what the Jakarta EE 11 Platform Specification Appendix A rules should be for EE schemas.

scottmarlow commented 1 week ago

An alternative could be to update ^ to https://jakarta.ee/xml/ns/jakartaee/web-app_6_0.xsd for EE 10 which in the case of the Tags component (standalone) TCK would help with EE 11 as well.

@gurunrao what do you think about further test changes to update the ^ schema references for EE 10 as an alternative?

Sadly, we currently have zero https://jakarta.ee/xml/ns/jakartaee/web-app_6_0.xsd use in the EE 10 Platform TCK. But we do have use of https://jakarta.ee/xml/ns/jakartaee/web-app_5_0.xsd. The concern about web-app_6_0.xsd is that some implementations might not support that.

I'll work on a pull request to switch to https://jakarta.ee/xml/ns/jakartaee/web-app_5_0.xsd.

scottmarlow commented 1 week ago

https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/issues_1313_10.0.x_part2/2/testReport/ shows the errors that we get testing this change.

The first error is com.sun.ts.tests.jstl.compat.onedotzero.JSTLClient.negativeScriptFreeTlvNoDeclTest

The Platform TCK build is at https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/issues_1313_10.0.x_part2/2/artifact/jakartaeetck-bundles/jakartaeetck-10.0.5.zip

gurunrao commented 1 week ago

https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/issues_1313_10.0.x_part2/2/testReport/ shows the errors that we get testing this change.

The first error is com.sun.ts.tests.jstl.compat.onedotzero.JSTLClient.negativeScriptFreeTlvNoDeclTest

The Platform TCK build is at https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/issues_1313_10.0.x_part2/2/artifact/jakartaeetck-bundles/jakartaeetck-10.0.5.zip

The deployment descriptor has elements not supported by new Schema, hence the parsing error.

scottmarlow commented 1 week ago

@gurunrao https://gist.github.com/scottmarlow/f2ae70d486d8fab69f41dfc0c022bcc1 is a list of EE 10 Platform TCK source files that reference sun-application-client_5_0-0.dtd

scottmarlow commented 1 week ago

@gurunrao as a follow up to https://github.com/jakartaee/platform-tck/pull/1335#issuecomment-2186867888, I added more changes to remove the display-name field and started another test run via https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/issues_1313_10.0.x_part2/3. If that passes and if someone could update the sun-application-client_5_0-0.dtd references, we might be able to avoid excluding the challenged tests.

gurunrao commented 1 week ago

sun-application-client_5_0-0.dtd

@scottmarlow - update for sun-application-client_5_0-0.dtd references will be with glassfish-application-client_6_0-1.dtd. document reference can be found at https://glassfish.org/docs/5.1.0/application-deployment-guide/dd-files.html. If community agrees with the change, we should be able to proceed with the update.

scottmarlow commented 1 week ago

sun-application-client_5_0-0.dtd

@scottmarlow - update for sun-application-client_5_0-0.dtd references will be with glassfish-application-client_6_0-1.dtd. document reference can be found at https://glassfish.org/docs/5.1.0/application-deployment-guide/dd-files.html. If community agrees with the change, we should be able to proceed with the update.

Please do proceed with the update. Thank you!

gurunrao commented 1 week ago

sun-application-client_5_0-0.dtd

@scottmarlow - update for sun-application-client_5_0-0.dtd references will be with glassfish-application-client_6_0-1.dtd. document reference can be found at https://glassfish.org/docs/5.1.0/application-deployment-guide/dd-files.html. If community agrees with the change, we should be able to proceed with the update.

Please do proceed with the update. Thank you!

sun-application-client_5_0-0.dtd is defined as vendor specific schema and is not part of Jakarta EE schema [ https://jakarta.ee/xml/ns/jakartaee/#8 ], Hence I have removed the schema reference from the challenge description. Jakarta EE Schema list can be found at https://jakarta.ee/xml/ns/jakartaee/#8