Changeset 33045

Timestamp:

2019-05-03T21:46:42+12:00 (5 years ago)

Author:

ak19

Message:

1. Dr Bainbridge's fix for the Windows encoding issue when online doc editing Title meta containing non-ASCII/non-basic unicode chars, was to URL encode the QUERY_STRING so that CGI.pm would then do the right thing. This was because the problem had turned out not to be env vars on Windows, which could set and recall unicode and win-1252 chars just fine and therefore retained what was entered. The problem was that on Windows, the perl did not get the actual chars transmitted in the case of UTF-8 whereas win-1252 was received looking apparently like a unicode codepoint, but then in the latter case the utf82unicode call in metadataaction would then clobber the codepoint in attempting to do utf82unicode on it. On linux, perl happened to receive the chars as utf8-encoded bytes and so utf82unicode worked (to make them unicode aware strings?). The real problem was that it could go wrong in different ways on windows, since utf8 chars weren't even received properly by perl/CGI, so we didn't even need to start worrying about them getting sometimes clobbered in metadataaction. URL encoding the QUERY_STRING was meant to solve this. Except that URL encoding the whole QUERY_STRING made CGI.pm choke on the equals signs between param name and param value and possibly other chars. I don't know why. I found that URL encoding just the param values in the QUERY_STRING works, so I am committing that. 2. Renaming the recently introduced string2hex() in JavaScript to debug_unicode_string and stringToHex() in Java to debugUnicodeString() to be more consistent with the perl variant, debug_unicode_string. Also like in the perl, the JavaScript and Java now print the unicode value inside curly braces for better legibility. 3. Leaving in some commented out encoding debugging statements in the Java and JavaScript code, but not committing the debugging on the perl side. 4. Some further improvements to overloaded functions in GSXML using debug_unicode_string for converting XML elements or printing them to logs.

Location:

main/trunk/greenstone3

Files:

• src/java/org/greenstone/gsdl3/build/GS2PerlConstructor.java (modified) (4 diffs)
• src/java/org/greenstone/gsdl3/service/GS2Construct.java (modified) (4 diffs)
• src/java/org/greenstone/gsdl3/util/GSXML.java (modified) (3 diffs)
• src/java/org/greenstone/util/Misc.java (modified) (2 diffs)
• web/interfaces/default/js/javascript-global-functions.js (modified) (3 diffs)

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/GS2PerlConstructor.java

@@ -20 +20 @@
 import java.io.InputStream;
 import java.io.IOException;
+//import java.io.UnsupportedEncodingException;
+//import java.net.URLEncoder;
+//import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Vector;
+
 
 import org.apache.log4j.*;
@@ -325 +329 @@
         command.add(cgi_directory + File.separator + "metadata-server.pl");
         
+        
         // Need to set QUERY_STRING and REQUEST_METHOD=GET in environment
         // Also set GS3_AUTHENTICATED, to allow running metadata-server.pl with mod (set and remove) commands
         // http://www.cgi101.com/class/ch3/text.html
+        
+        // And need to ensure that special characters won't get clobbered on Windows by perl/CGI.pm (https://www.nntp.perl.org/group/perl.perl5.porters/2016/10/msg240120.html), 
+        // URL encode the query_string, as at https://stackoverflow.com/questions/10786042/java-url-encoding-of-query-string-parameters
+        
+        // perl/CGI.pm doesn't like us URL encoding the entire query string such as the equal sign between each paramName and paramValue.
+        // So we URL encode each paramValue separately, which is done in GS2Construct.java::runCommand()
+        /*
+        String old_query_string = this.query_string;
+        try{
+            this.query_string = URLEncoder.encode(this.query_string, StandardCharsets.UTF_8.name());
+            //this.query_string = this.query_string.replace("+","%2B"); // https://stackoverflow.com/questions/1211229/in-a-url-should-spaces-be-encoded-using-20-or
+        } catch(UnsupportedEncodingException uee) {
+            logger.warn("**** Unable to encode query_string in UTF-8, so attempting to continue with the unencoded value of query_string");
+            this.query_string = old_query_string; 
+        }
+        */
+        
         String[] envvars = {
             "QUERY_STRING=" + this.query_string,
@@ -464 +486 @@
     
     ///logger.info("### Running logged command = " + command_str);
-
+    /*
+    // USEFUL DEBUGGING WHEN USING DOC EDITOR TO MODIFY ex.Title META THAT CONTAINS NON-BASIC ASCII CHARS
+    logger.error("### Running logged command = " + command_str);
+    logger.error("### DEBUG Running logged command = " + Misc.stringToHex(command_str));
+    if(envvars != null) {
+        for(int i = 0; i < envvars.length; i++) {
+            logger.error("### envvar = " + envvars[i]);
+            logger.error("### DEBUG envvar = " + Misc.stringToHex(envvars[i]));
+        }
+    }
+    */
+    
     // This is where we create and run our perl process safely
     SafeProcess perlProcess = createPerlProcess(command, envvars, dir); //  dir can be null
@@ -755 +788 @@
             if(this.source == SafeProcess.STDERR) {
                 System.err.println("STDERR: " + line);
+                logger.info("@@@@@@@@ STDERR: " + line );
             } else {
                 System.err.println("STDOUT: " + line);
+                logger.info("@@@@@@@@ STDOUT: " + line );
             }   
             

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/GS2Construct.java

@@ -23 +23 @@
 import java.io.FileWriter;
 import java.io.Serializable;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.Iterator;
@@ -746 +749 @@
 
         Element param_list = (Element) GSXML.getChildByTagName(request, GSXML.PARAM_ELEM + GSXML.LIST_MODIFIER);
+        
+        //GSXML.elementToLogAsString("###      Extracted param_list: ", param_list, true);
+        //GSXML.elementToLogAsUnicodeDebugString("### DEBUG Extracted param_list: ", param_list, true);
+        
         HashMap<String, Serializable> params = GSXML.extractParams(param_list, false);
 
@@ -838 +845 @@
                 String paramvalue = (String) entry.getValue();
 
-                querystring.append(paramname + "=" + paramvalue);
+                // And need to ensure that special characters won't get clobbered on Windows by perl/CGI.pm (https://www.nntp.perl.org/group/perl.perl5.porters/2016/10/msg240120.html), 
+                // URL encode the query_string, as at https://stackoverflow.com/questions/10786042/java-url-encoding-of-query-string-parameters
+        
+                // perl/CGI.pm doesn't like us URL encoding the entire query string such as the equal sign between each paramName and paramValue.
+                // So we URL encode each paramValue separately, which is done in GS2Construct.java::runCommand()
+                querystring.append(paramname + "=" + urlEncodeValue(paramname, paramvalue));
                 if (i.hasNext()) {
                     querystring.append("&");
@@ -882 +894 @@
     //************************
 
+    private String urlEncodeValue(String paramName, String paramVal) {
+        String oldParamVal = paramVal;
+        try{
+            paramVal = URLEncoder.encode(paramVal, StandardCharsets.UTF_8.name());          
+        } catch(UnsupportedEncodingException uee) {
+            logger.warn("**** Unable to encode query_string param " + paramName + " in UTF-8, so attempting to continue with its unencoded value."); // don't output param value to log, in case of sensitive data?
+            paramVal = oldParamVal; 
+        }
+        return paramVal;
+    }
+    
     /** parse the collect directory and return a list of collection names */
     protected String[] getCollectionList()

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/util/GSXML.java

@@ -1624 +1624 @@
     }
 
-    public static void elementToLogAsString(Element e, boolean indent)
-    {
-        String str = elementToString(e, indent);
+    private static void elementToLogAsString(String prefix, Element e, boolean indent, boolean debugEncoding)
+    {
+        String str = prefix + "\n" + elementToString(e, indent, debugEncoding);
         System.err.println(str);
-        logger.error(str);
+        logger.info(str);
+    }
+    
+    // hex/unicode codepoint used only for those chars that are beyond printable/basic ASCII
+    public static void elementToLogAsUnicodeDebugString(String prefix, Element e, boolean indent)
+    {
+        elementToLogAsString(prefix, e, indent, true);
+    }
+    
+    public static void elementToLogAsString(String prefix, Element e, boolean indent)
+    {
+        elementToLogAsString(prefix, e, indent, false);
     }
 
     // pass in debugEncoding=true to investigate encoding issues. This function will then return non-basic ASCII characters in hex
-    public static String elementToString(Element e, boolean indent, boolean debugEncoding)
+    private static String elementToString(Element e, boolean indent, boolean debugEncoding)
     {
         String str = "";
@@ -1652 +1663 @@
             
             // if debugging encoding issues, then encode unicode code pts as hex for all but non-alphanumeric and space/tab/newline chars
-            if(debugEncoding) str = Misc.stringToHex(str);
+            if(debugEncoding) str = Misc.debugUnicodeString(str);
         }
         catch (Exception ex)
@@ -1667 +1678 @@
     {
         return elementToString(e, indent, false);
+    }
+    
+    // hex/unicode codepoint used only for those chars that are beyond printable/basic ASCII
+    public static String elementToUnicodeDebugString(Element e, boolean indent)
+    {
+        return elementToString(e, indent, true);
     }
     

main/trunk/greenstone3/src/java/org/greenstone/util/Misc.java

@@ -58 +58 @@
     
     
-    // Debugging function to print a string's non-basic chars in hex
+    // Debugging function to print a string's non-basic chars in hex, so stringToHex on all non-basic and non-printable ASCII
+    // Dr Bainbridge said that printing anything with charCode over 128 in hex is okay, but I'd already made extra allowances for non-printable ASCII
     // Based on https://stackoverflow.com/questions/923863/converting-a-string-to-hexadecimal-in-java
-    public static String stringToHex(String str) {
+    public static String debugUnicodeString(String str) {
       String result = "";
       for(int i = 0; i < str.length(); i++) {
@@ -67 +68 @@
             // ASCII table: https://cdn.sparkfun.com/assets/home_page_posts/2/1/2/1/ascii_table_black.png
             // If the unicode character code pt is less than the ASCII code for space and greater than for tilda, let's display the char in hex (x0000 format)
-            if((charCode >= 20 && charCode <= 126) || charCode == 9 || charCode == 10 || charCode == 13) { // space to tilda, TAB, LF, CR are printable
+            if((charCode >= 20 && charCode <= 126) || charCode == 9 || charCode == 10 || charCode == 13) { // space, tilda, TAB, LF, CR are printable, leave them in for XML element printing
                 result += str.charAt(i);
             } else {
-                result += "x" + String.format("%04x", charCode);
+                result += "x{" + String.format("%04x", charCode) + "}"; // looks like: x{4-char-codepoint}
             }
       }

main/trunk/greenstone3/web/interfaces/default/js/javascript-global-functions.js

@@ -13 +13 @@
 }
 
-// Debugging function to print a string's non-basic chars in hex
+// Debugging function to print a string's non-basic chars in hex. So does string2hex on all non-basic and non-printable ASCII chars
+// Dr Bainbridge said that printing anything with charCode over 128 in hex is okay, but I'd already made extra allowances for non-printable ASCII
 // Based on https://stackoverflow.com/questions/36637146/javascript-encode-string-to-hex/36637293
 // https://stackoverflow.com/questions/21647928/javascript-unicode-string-to-hex
-gs.functions.string2hex = function(str) {
+gs.functions.debug_unicode_string = function(str) {
     var hex, i;
 
@@ -26 +27 @@
         if(charcode < 20 || charcode > 126) { //doesn't work: if(str.charAt(i) < ' ' || str.charAt(i) > '~') {
             hex = charcode.toString(16);
-            result += "x" + ("000"+hex).slice(-4);
+            result += "x{" + ("000"+hex).slice(-4) + "}"; // looks like: x{4-char-codepoint}
         }
         else {
@@ -785 +786 @@
 gs.functions.setArchivesMetadata = function(collection, site, documentID, metadataName, metadataPosition, metadataValue, prevMetadataValue, metamode, successResponseFunction, errorResponseFunction)
 {
+    if(metadataValue) console.log("metaval: " + metadataValue + " | " + gs.functions.debug_unicode_string(metadataValue)); //metadataValue.hexEncode()
+    if(prevMetadataValue) console.log("prevmetaval: " + prevMetadataValue + " | " + gs.functions.debug_unicode_string(prevMetadataValue));
+    
     if( typeof errorResponseFunction === 'undefined' ) { errorResponseFunction = null; } // force error callback to be defined: either null or has value