From: Jeff Genovy <29107334+jefgen@users.noreply.github.com>
Date: Wed, 8 Aug 2018 19:13:42 +0000 (-0700)
Subject: ICU-13686 ICU4C: Prevent double file separators on data file paths. (#42)
X-Git-Tag: release-63-rc~126
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4e49234da9f3db0dcce6ede85897adaf8ccd40ba;p=icu

ICU-13686 ICU4C: Prevent double file separators on data file paths. (#42)

ICU very often ends up with double separators in the file paths used to open files. This change prevents extra slashes on data file paths.
- The feedback from Markus was that the suffix shouldn't really start with a separator.
- It turns out that the suffix can be used for either a file extension (like .dat or .res) or for an actual item name as well (like ibm-5348_P100-1997.cnv).
- If the suffix is an extension then we don't want to append the separator at all. We only want to append it if the suffix is an item name.
- Also use a StringPiece to save repeated implicit calls to uprv_strlen.
---

diff --git a/icu4c/source/common/udata.cpp b/icu4c/source/common/udata.cpp
index 3cb8863f6c3..1905804857f 100644
--- a/icu4c/source/common/udata.cpp
+++ b/icu4c/source/common/udata.cpp
@@ -418,7 +418,8 @@ private:
     const char *path;                              /* working path (u_icudata_Dir) */
     const char *nextPath;                          /* path following this one */
     const char *basename;                          /* item's basename (icudt22e_mt.res)*/
-    const char *suffix;                            /* item suffix (can be null) */
+
+    StringPiece suffix;                            /* item suffix (can be null) */
 
     uint32_t    basenameLen;                       /* length of basename */
 
@@ -432,13 +433,15 @@ private:
 };
 
 /**
- * @param iter  The iterator to be initialized. Its current state does not matter. 
- * @param path  The full pathname to be iterated over.  If NULL, defaults to U_ICUDATA_NAME 
- * @param pkg   Package which is being searched for, ex "icudt28l".  Will ignore leave directories such as /icudt28l 
- * @param item  Item to be searched for.  Can include full path, such as /a/b/foo.dat 
- * @param suffix  Optional item suffix, if not-null (ex. ".dat") then 'path' can contain 'item' explicitly.
- *               Ex:   'stuff.dat' would be found in '/a/foo:/tmp/stuff.dat:/bar/baz' as item #2.   
- *                     '/blarg/stuff.dat' would also be found.
+ * @param iter    The iterator to be initialized. Its current state does not matter.
+ * @param inPath  The full pathname to be iterated over.  If NULL, defaults to U_ICUDATA_NAME 
+ * @param pkg     Package which is being searched for, ex "icudt28l".  Will ignore leaf directories such as /icudt28l 
+ * @param item    Item to be searched for.  Can include full path, such as /a/b/foo.dat 
+ * @param inSuffix  Optional item suffix, if not-null (ex. ".dat") then 'path' can contain 'item' explicitly.
+ *             Ex:   'stuff.dat' would be found in '/a/foo:/tmp/stuff.dat:/bar/baz' as item #2.   
+ *                   '/blarg/stuff.dat' would also be found.
+ *  Note: inSuffix may also be the 'item' being searched for as well, (ex: "ibm-5348_P100-1997.cnv"), in which case 
+ *        the 'item' parameter is often the same as pkg. (Though sometimes might have a tree part as well, ex: "icudt62l-curr").
  */
 UDataPathIterator::UDataPathIterator(const char *inPath, const char *pkg,
                                      const char *item, const char *inSuffix, UBool doCheckLastFour,
@@ -566,7 +569,7 @@ const char *UDataPathIterator::next(UErrorCode *pErrorCode)
 
         if(checkLastFour == TRUE && 
            (pathLen>=4) &&
-           uprv_strncmp(pathBuffer.data() +(pathLen-4), suffix, 4)==0 && /* suffix matches */
+           uprv_strncmp(pathBuffer.data() +(pathLen-4), suffix.data(), 4)==0 && /* suffix matches */
            uprv_strncmp(findBasename(pathBuffer.data()), basename, basenameLen)==0  && /* base matches */
            uprv_strlen(pathBasename)==(basenameLen+4)) { /* base+suffix = full len */
 
@@ -602,8 +605,13 @@ const char *UDataPathIterator::next(UErrorCode *pErrorCode)
             /* + basename */
             pathBuffer.append(packageStub.data()+1, packageStub.length()-1, *pErrorCode);
 
-            if(*suffix)  /* tack on suffix */
+            if (!suffix.empty())  /* tack on suffix */
             {
+                if (suffix.length() > 4) {
+                    // If the suffix is actually an item ("ibm-5348_P100-1997.cnv") and not an extension (".res")
+                    // then we need to ensure that the path ends with a separator.
+                    pathBuffer.ensureEndsWithFileSeparator(*pErrorCode);
+                }
                 pathBuffer.append(suffix, *pErrorCode);
             }
         }
@@ -1252,7 +1260,8 @@ doOpenChoice(const char *path, const char *type, const char *name,
         tocEntryName.append(".", *pErrorCode).append(type, *pErrorCode);
         tocEntryPath.append(".", *pErrorCode).append(type, *pErrorCode);
     }
-    tocEntryPathSuffix = tocEntryPath.data()+tocEntrySuffixIndex; /* suffix starts here */
+    // The +1 is for the U_FILE_SEP_CHAR that is always appended above.
+    tocEntryPathSuffix = tocEntryPath.data() + tocEntrySuffixIndex + 1; /* suffix starts here */
 
 #ifdef UDATA_DEBUG
     fprintf(stderr, " tocEntryName = %s\n", tocEntryName.data());