Re: [PATCH v3 3/4] enter_repo: do not modify input

On Tue, Oct 4, 2011 at 7:55 PM, Phil Hord [off-list ref] wrote:

On Thu, Sep 29, 2011 at 4:59 PM, Erik Faye-Lund [off-list ref] wrote:

quoted

entr_repo(..., 0) currently modifies the input to strip away
trailing slashes. This means that we some times need to copy the
input to keep the original.

I'm also modifying enter_repo() so I'm looking a bit closer at this patch now.

quoted

Change it to unconditionally copy it into the used_path buffer so
we can safely use the input without having to copy it.

Signed-off-by: Erik Faye-Lund <redacted>
---

[...]

quoted

 */
-char *enter_repo(char *path, int strict)
+const char *enter_repo(const char *path, int strict)
 {
       static char used_path[PATH_MAX];
       static char validated_path[PATH_MAX];

@@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)

               };
               int len = strlen(path);
               int i;
-               while ((1 < len) && (path[len-1] == '/')) {
-                       path[len-1] = 0;
+               while ((1 < len) && (path[len-1] == '/'))
                       len--;
-               }
+
               if (PATH_MAX <= len)
                       return NULL;
-               if (path[0] == '~') {
-                       char *newpath = expand_user_path(path);
+               strncpy(used_path, path, len);

When len < strlen(path), this will will leave used_path unterminated.

Good catch, thanks!

quoted

+
+               if (used_path[0] == '~') {
+                       char *newpath = expand_user_path(used_path);
                       if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
                               free(newpath);
                               return NULL;

@@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)

                        * anyway.
                        */
                       strcpy(used_path, newpath); free(newpath);
-                       strcpy(validated_path, path);
-                       path = used_path;
+                       strcpy(validated_path, used_path);

The point of 'validated_path' is to keep the original unmolested,
unexpanded path string (plus DWIM suffix), but here you've just
replaced validated_path with a copy of the expanded_user_path.  On the
other hand, we seem always to strcpy(validated_path , path), so we
might as well get that done up-front.

Yeah, that's probably better.