Konstantin,

On 1/15/24 09:30, Konstantin Kolinko wrote:
пн, 15 янв. 2024 г. в 13:16, <ma...@apache.org>:

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
      new bee714eb1d Fix logic bug spotted by Coverity Scan
bee714eb1d is described below

commit bee714eb1dabfcc07bf410e1145f6c580f14539d
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Jan 15 10:11:59 2024 +0000

     Fix logic bug spotted by Coverity Scan
---
  .../apache/tomcat/util/digester/ServiceBindingPropertySource.java    | 2 +-
  webapps/docs/changelog.xml                                           | 5 +++++
  2 files changed, 6 insertions(+), 1 deletion(-)

diff --git 
a/java/org/apache/tomcat/util/digester/ServiceBindingPropertySource.java 
b/java/org/apache/tomcat/util/digester/ServiceBindingPropertySource.java
index 89617c9cfb..bd06630f01 100644
--- a/java/org/apache/tomcat/util/digester/ServiceBindingPropertySource.java
+++ b/java/org/apache/tomcat/util/digester/ServiceBindingPropertySource.java
@@ -116,7 +116,7 @@ public class ServiceBindingPropertySource implements 
IntrospectionUtils.Property
              int length = bytes.length;

              if (chomp) {
-                if(length > 1 && bytes[length - 2] == '\r' && bytes[length - 
2] == '\n') {
+                if(length > 1 && bytes[length - 2] == '\r' && bytes[length - 
1] == '\n') {
                      length -= 2;
                  } else if (length > 0) {
                      byte c = bytes[length - 1];
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 6e9ffb917c..aa7fce0034 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -120,6 +120,11 @@
          that is no longer included in the JAR. Based on pull request
          <pr>684</pr> by Jendrik Johannes. (markt)
        </fix>
+      <fix>
+        Fix ServiceBindingPropertySource so that trailing <code>\r\n</code>
+        sequences are correctly removed from files containing property values
+        when configured to do so. Bug identified by Coverity Scan. (markt)
+      </fix>
      </changelog>
    </subsection>
    <subsection name="Coyote">

Reviewing the code of ServiceBindingPropertySource,

1. It removes only the last end-of-line sequence (one or two characters).
It could be rewritten with a loop to remove any \n \r characters that
are found at the end.

This was intentional (only trim the final newline) but could be updated if the team thinks it makes sense to do so.

Maybe text-editing programs append a trailing newline at the end of text files unless you take special steps to prevent it from happening. It's usually easy to limit the trailing newlines to a single one.

2. The code later uses "return new String(bytes, 0, length);"
to create a String, without specifying any encoding.

This was intentional as well, as the file is originally read without specifying the encoding. Allowing an admin to specify the encoding would have expanded the configuration for ServiceBrindingPropertySource and my intent at the time was to make the fewest number of changes possible.

I guess that the Platform default encoding nowadays is UTF-8.
Or maybe it makes sense to use the default encoding, as people edit
those files, using whatever default encoding they prefer.

+1

A PropertySource is specified using a system property. Specifying the encoding for a file-based PropertySource would mean having another system property or a more complicated configuration scheme for PropertySource.

I'm happy to entertain ideas for improvement.

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to