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

Change check for modifications of saveservice.properties from $Revision to sha1 sum of file #3691

Closed
asfimport opened this issue Nov 11, 2015 · 3 comments

Comments

@asfimport
Copy link
Collaborator

@FSchumacher (Bug 58601):
Currently SaveService checks the integrity of saveservice.properties by reading $Revision, which is a svn feature. When tests are done in a git repo, the tests will fail.

This can be mitigated, by computing a sha1 checksum of saveservice.properties and use that for comparison with an expected value stored in SaveService.

Created attachment 0001-Change-usage-of-number-for-checking-modifications-of.patch: Change usage of modification check to sha1 sum

0001-Change-usage-of-number-for-checking-modifications-of.patch
From 46a6ade523d94890725e773ca865d4b7e892549f Mon Sep 17 00:00:00 2001
From: Felix Schumacher <felix.schumacher@internetallee.de>
Date: Wed, 11 Nov 2015 16:48:35 +0100
Subject: [PATCH] Change usage of  number for checking modifications of
 saveservice.properties to sha1 checksum of the file

---
 bin/saveservice.properties                       |  6 +--
 src/core/org/apache/jmeter/save/SaveService.java | 49 ++++++++++++++++--------
 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/bin/saveservice.properties b/bin/saveservice.properties
index 11e01df..3f29133 100644
--- a/bin/saveservice.properties
+++ b/bin/saveservice.properties
@@ -34,8 +34,8 @@
 # Please keep the entries in alphabetical order within the sections
 # to reduce the likelihood of duplicates
 #
-# version number of this file (automatically generated by SVN)
-_file_version=$Revision$
+# version number of this file is now computed by a sha1 sum, so no need for
+# an explicit _file_version property anymore
 #
 # Conversion version (for JMX output files)
 # Must be updated if the file has been changed since the previous release
@@ -381,4 +381,4 @@ _org.apache.jmeter.save.converters.TestResultWrapperConverter=collection
 _org.apache.jmeter.save.ScriptWrapperConverter=mapping
 #
 #	Remember to update the _version entry
-#
\ No newline at end of file
+#
diff --git a/src/core/org/apache/jmeter/save/SaveService.java b/src/core/org/apache/jmeter/save/SaveService.java
index 09e9071..55f9708 100644
--- a/src/core/org/apache/jmeter/save/SaveService.java
+++ b/src/core/org/apache/jmeter/save/SaveService.java
@@ -29,6 +29,8 @@ import java.io.OutputStreamWriter;
 import java.io.Writer;
 import java.lang.reflect.InvocationTargetException;
 import java.nio.charset.Charset;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -180,10 +182,10 @@ public class SaveService {
     static final String PROPVERSION = "2.9";// Expected version $NON-NLS-1$
 
     // Internal information only
-    private static String fileVersion = ""; // read from saveservice.properties file// $NON-NLS-1$
-    // Must match Revision id value in saveservice.properties, 
+    private static String fileVersion = ""; // computed from saveservice.properties file// $NON-NLS-1$
+    // Must match the sha1 checksum of the file saveservice.properties,
     // used to ensure saveservice.properties and SaveService are updated simultaneously
-    static final String FILEVERSION = "1709921"; // Expected value $NON-NLS-1$
+    static final String FILEVERSION = "99ce1dfd503fc025603a17c23e25f2d623981cc5"; // Expected value $NON-NLS-1$
 
     private static String fileEncoding = ""; // read from properties file// $NON-NLS-1$
 
@@ -221,9 +223,34 @@ public class SaveService {
         }
         return nameMap;
     }
+
+    private static String getChecksumForPropertiesFile()
+            throws NoSuchAlgorithmException, IOException {
+        FileInputStream fis = null;
+        MessageDigest md = MessageDigest.getInstance("SHA1");
+        try {
+            fis = new FileInputStream(JMeterUtils.getJMeterHome()
+                    + JMeterUtils.getPropDefault(SAVESERVICE_PROPERTIES,
+                            SAVESERVICE_PROPERTIES_FILE));
+            byte[] readBuffer = new byte[8192];
+            int bytesRead;
+            while ((bytesRead = fis.read(readBuffer)) != -1) {
+                md.update(readBuffer, 0, bytesRead);
+            }
+        } finally {
+            JOrphanUtils.closeQuietly(fis);
+        }
+        return JOrphanUtils.baToHexString(md.digest());
+    }
     private static void initProps() {
         // Load the alias properties
         try {
+            fileVersion = getChecksumForPropertiesFile();
+        } catch (IOException | NoSuchAlgorithmException e) {
+            log.fatalError("Can't compute checksum for saveservice properties file", e);
+            throw new JMeterError("JMeter requires the checksum of saveservice properties file to continue", e);
+        }
+        try {
             Properties nameMap = loadProperties();
             // now create the aliases
             for (Map.Entry<Object, Object> me : nameMap.entrySet()) {
@@ -237,8 +264,8 @@ public class SaveService {
                         propertiesVersion = val;
                         log.info("Using SaveService properties version " + propertiesVersion);
                     } else if (key.equalsIgnoreCase("_file_version")) { // $NON-NLS-1$
-                            fileVersion = extractVersion(val);
-                            log.info("Using SaveService properties file version " + fileVersion);
+                        log.info("SaveService properties file version is now computed by a checksum,"
+                                + "the property _file_version is not used anymore and can be removed.");
                     } else if (key.equalsIgnoreCase("_file_encoding")) { // $NON-NLS-1$
                         fileEncoding = val;
                         log.info("Using SaveService properties file encoding " + fileEncoding);
@@ -384,18 +411,6 @@ public class SaveService {
 
     private static boolean versionsOK = true;
 
-    // Extract version digits from String of the form #Revision: n.mm #
-    // (where # is actually $ above)
-    private static final String REVPFX = "$Revision: ";
-    private static final String REVSFX = " $"; // $NON-NLS-1$
-
-    private static String extractVersion(String rev) {
-        if (rev.length() > REVPFX.length() + REVSFX.length()) {
-            return rev.substring(REVPFX.length(), rev.length() - REVSFX.length());
-        }
-        return rev;
-    }
-
 //  private static void checkVersion(Class clazz, String expected) {
 //
 //      String actual = "*NONE*"; // $NON-NLS-1$
-- 
1.9.1

OS: Windows CE

@asfimport
Copy link
Collaborator Author

@FSchumacher (migrated from Bugzilla):
Date: Sat Nov 14 13:48:09 2015
New Revision: 1714325

URL: http://svn.apache.org/viewvc?rev=1714325&view=rev
Log:
Change check for modifications of saveservice.properties from $Revision to sha1 sum of file.

#3691

Modified:
jmeter/trunk/bin/saveservice.properties
jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java
jmeter/trunk/xdocs/changes.xml

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
I think there is a problem with the checksum calculation.

The saveservice.properties file has native eol, so will be different on different OSes.

It should be possible to fix this by reading lines rather than bytes.

@asfimport
Copy link
Collaborator Author

@FSchumacher (migrated from Bugzilla):
Date: Sun Nov 15 13:57:19 2015
New Revision: 1714452

URL: http://svn.apache.org/viewvc?rev=1714452&view=rev
Log:
Ignore newline character when computing the sha1 sum of saveservice.properties.

Followup to r1714325, since sebb pointed out, that the file could have different
line endings when checked out on different OSes.

#3691

Modified:
jmeter/trunk/bin/saveservice.properties
jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant