[libvirt] [PATCH] storage: Sanitize pool target paths

[Eric Blake]

On 05/20/2010 10:04 AM, Cole Robinson wrote:
> Spurious / in a pool target path makes life difficult for apps using the
> GetVolByPath, and doing other path based comparisons with pools. This
> has caused a few issues for virt-manager users:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=494005
> https://bugzilla.redhat.com/show_bug.cgi?id=593565
> 
> Add a new util API which removes spurious /, virFileSanitizePath. Sanitize
> target paths when parsing pool XML, and for paths passed to GetVolByPath.

In general, I like the idea, whether or not we should be moving the
sanitization into a gnulib function.  But I don't think it's ready for
an ack quite yet.

> index e937d39..8f86ed6 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -1921,6 +1921,41 @@ int virFileAbsPath(const char *path, char **abspath)
>      return 0;
>  }
>  
> +/* Remove spurious / characters from a path. The result must be freed */
> +char *
> +virFileSanitizePath(const char *path)
> +{
> +    const char *cur = path;
> +    char *cleanpath;
> +    int idx = 0;
> +
> +    cleanpath = strdup(path);
> +    if (!cleanpath) {
> +        virReportOOMError();
> +        return NULL;
> +    }
> +
> +    /* Sanitize path in place */
> +    while (*cur != '\0') {
> +        if (*cur == '/') {
> +            /* Skip all extra / */
> +            while (*++cur == '/')
> +                continue;

POSIX says that you must be careful of "//file" - you cannot blindly
simplify to "/file" (case in point - cygwin).  But POSIX also says that
"///file" sanitizes to "/file".

> +
> +            /* Don't add a trailing / */
> +            if (*cur == '\0')
> +                break;
> +
> +            cleanpath[idx++] = '/';
> +        }
> +
> +        cleanpath[idx++] = *cur++;
> +    }
> +    cleanpath[idx] = '\0';
> +
> +    return cleanpath;

Sanitizing "a/b/../c" to "a/c" might not always be the right thing (what
if a is a symlink?), but it would also be safe to sanitize "a/./b" to "a/b".

> +}
> +
>  /* Like strtol, but produce an "int" result, and check more carefully.
>     Return 0 upon success;  return -1 to indicate failure.
>     When END_PTR is NULL, the byte after the final valid digit must be NUL.
> diff --git a/src/util/util.h b/src/util/util.h
> index 6bf6bcc..abc2688 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -118,6 +118,8 @@ char *virFindFileInPath(const char *file);
>  
>  int virFileExists(const char *path);
>  
> +char *virFileSanitizePath(const char *path);

Mark this with ATTRIBUTE_NONNULL(1).

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100520/9c364e04/attachment-0001.sig>