bugfix for issue #11 (skip xml entities in filter method)

[jo-kl]

No description provided.

[ste-gr]

I think the test-case "123Ü456" will not work. Could you also add this case please.

[ste-gr]

The test-case "123&456;789" should return "123&456;789" because the "NameStartChar" is not allowed to begin with an number. See https://www.w3.org/TR/xml/#NT-Name

[ste-gr]

When unchanged, it's not needed to set filtered.

[ste-gr]

See also the suggested test-cases.

Suggested change

	protected static Pattern XML_ENTITY_PATTERN = Pattern.compile("&(?:[a-z\\d]+|#\\d+|#x[a-f\\d]+);");
	protected static Pattern XML_ENTITY_PATTERN = Pattern.compile("&(?:[a-zA-Z][a-zA-Z\\d]*|#\\d+|#x[a-fA-F\\d]+);");

[ste-gr]

Please delete blanks after ( and before ), thanks.

Suggested change

	if ( isStartOfXmlEntity(value,i) ) {
	if (isStartOfXmlEntity(value,i)) {

[ste-gr]

Suggested change

	if ( matcher.find() && matcher.start() == 0 ) {
	return matcher.find() && matcher.start() == 0;

[TimCincotta]

Hello, I have interest in working on this repository and assisting with pushing it over the finish line. Would it be possible to add the 'Discussions' option to this repository in GitHub? I would like to possibly have some discussions outside of specific issues and saw this as an option: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/enabling-or-disabling-github-discussions-for-a-repository [https://github.githubassets.com/images/modules/open_graph/github-logo.png]<https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/enabling-or-disabling-github-discussions-for-a-repository> Enabling or disabling GitHub Discussions for a repository - GitHub Docs<https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/enabling-or-disabling-github-discussions-for-a-repository> You can use GitHub Discussions in a repository as a place for your community to have conversations, ask questions, and post answers without scoping work in an issue. docs.github.com Regards, Tim Cincotta IT Architect – Senior FIS Payments Office: 414.815.1334 ***@***.******@***.***> FIS | Empowering the Financial World [cid:249df983-8fad-4f95-9219-0eea2c34a419] <https://www.facebook.com/FIStoday> [cid:9c970a3d-689f-4e6a-a65b-6bb3af911e02] <https://twitter.com/FISGlobal> [cid:bc6e49e1-d43c-4bee-8fe5-a3da78283966] <https://www.linkedin.com/company/fis> [50_3]

________________________________ From: Stefan Graff ***@***.***> Sent: Sunday, July 9, 2023 8:32 PM To: weblegacy/struts1 ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [weblegacy/struts1] bugfix for issue #11 (skip xml entities in filter method) (PR #12) @ste-gr requested changes on this pull request.

________________________________ In core/src/test/java/org/apache/struts/util/TestResponseUtils.java<#12 (comment)>:

+ public void tearDown() {

+ super.tearDown(); + } + + // ------------------------------------------------------- Individual Tests + // ---------------------------------------------------------- filter() + @test + public void testFilter() { + assertEquals("123&amp;456", ResponseUtils.filter("123&456")); + assertEquals("123&amp;456", ResponseUtils.filter("123&amp;456")); + assertEquals("123&#123;456", ResponseUtils.filter("123&#123;456")); + assertEquals("123&amp;#12a;456", ResponseUtils.filter("123&#12a;456")); + assertEquals("123&#x12a;456", ResponseUtils.filter("123&#x12a;456")); + assertEquals("123&amp;#x12ah;456", ResponseUtils.filter("123&#x12ah;456")); + assertEquals("123&lt;&gt;&quot;&#39;456", ResponseUtils.filter("123<>\"'456")); + } I think the test-case "123&Uuml;456" will not work. Could you also add this case please.

________________________________ In core/src/test/java/org/apache/struts/util/TestResponseUtils.java<#12 (comment)>:

+ @AfterEach

+ public void tearDown() { + super.tearDown(); + } + + // ------------------------------------------------------- Individual Tests + // ---------------------------------------------------------- filter() + @test + public void testFilter() { + assertEquals("123&amp;456", ResponseUtils.filter("123&456")); + assertEquals("123&amp;456", ResponseUtils.filter("123&amp;456")); + assertEquals("123&#123;456", ResponseUtils.filter("123&#123;456")); + assertEquals("123&amp;#12a;456", ResponseUtils.filter("123&#12a;456")); + assertEquals("123&#x12a;456", ResponseUtils.filter("123&#x12a;456")); + assertEquals("123&amp;#x12ah;456", ResponseUtils.filter("123&#x12ah;456")); + assertEquals("123&lt;&gt;&quot;&#39;456", ResponseUtils.filter("123<>\"'456")); The test-case "123&456;789" should return "123&amp;456;789" because the "NameStartChar" is not allowed to begin with an number. See https://www.w3.org/TR/xml/#NT-Name

________________________________ In core/src/main/java/org/apache/struts/util/ResponseUtils.java<#12 (comment)>:

@@ -82,8 +87,11 @@ public static String filter(String value) {

break; case '&': - filtered = "&amp;"; - + if ( isStartOfXmlEntity(value,i) ) { + filtered = "&"; // leave unchanged When unchanged, it's not needed to set filtered.

________________________________ In core/src/main/java/org/apache/struts/util/ResponseUtils.java<#12 (comment)>:

@@ -35,6 +37,9 @@

* $ */ public class ResponseUtils { + + protected static Pattern XML_ENTITY_PATTERN = Pattern.compile("&(?:[a-z\\d]+|#\\d+|#x[a-f\\d]+);"); See also the suggested test-cases. ⬇️ Suggested change - protected static Pattern XML_ENTITY_PATTERN = Pattern.compile("&(?:[a-z\\d]+|#\\d+|#x[a-f\\d]+);"); + protected static Pattern XML_ENTITY_PATTERN = Pattern.compile("&(?:[a-zA-Z][a-zA-Z\\d]*|#\\d+|#x[a-fA-F\\d]+);");

________________________________ In core/src/main/java/org/apache/struts/util/ResponseUtils.java<#12 (comment)>:

@@ -82,8 +87,11 @@ public static String filter(String value) {

break; case '&': - filtered = "&amp;"; - + if ( isStartOfXmlEntity(value,i) ) { Please delete blanks after ( and before ), thanks. ⬇️ Suggested change - if ( isStartOfXmlEntity(value,i) ) { + if (isStartOfXmlEntity(value,i)) {

________________________________ In core/src/main/java/org/apache/struts/util/ResponseUtils.java<#12 (comment)>:

@@ -120,6 +128,21 @@ public static String filter(String value) {

} /** + * Checks, if a given string contains a XML entity starting at a specified position + * @param str the string value to check + * @param startpos the index where the entity is expected to start + * @return <code>true</code> if a XML entity was found, <code>false</code> otherwise + */ + private static boolean isStartOfXmlEntity(String str, int startpos) { + Matcher matcher = XML_ENTITY_PATTERN.matcher(str.substring(startpos)); + if ( matcher.find() && matcher.start() == 0 ) { ⬇️ Suggested change - if ( matcher.find() && matcher.start() == 0 ) { + return matcher.find() && matcher.start() == 0; — Reply to this email directly, view it on GitHub<#12 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAXDQVTVMAYHRPOSD2BDYSDXPNLRNANCNFSM6AAAAAA2BJ2RX4>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***> The information contained in this message is proprietary and/or confidential. If you are not the intended recipient, please: (i) delete the message and all copies; (ii) do not disclose, distribute or use the message in any manner; and (iii) notify the sender immediately. In addition, please be aware that any message addressed to our domain is subject to archiving and review by persons other than the intended recipient. Thank you.

[ste-gr]

Hi Tim,

It would be nice to get some interesting new ideas and support to make Struts1 ready for JEE9+. I have opened the discussion [#14].

Currently it has the following status:

• I have started to change the namespaces from javax to jakarta.
• Unfortunately, the following dependencies are not yet JEE9+ compatible:
  • commons-fileupload (there is now version 2, which is JEE9+ compatible).
  • commons-chain (I'm working on this right now, very old)
  • tiles-request-servlet (I still want to have a look at the current solution, write a small doc and then publish it)

Nice to have, but not necessary:

• tlddoc - Tag Library Documentation Generator
  • Update to taglib 3.0 (from 2.1)
  • Testcases
• taglib-maven-plugin - Update to new tlddoc

Regards
Stefan