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

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).