Alexander Wels has uploaded a new change for review. Change subject: userportal,webadmin: fix comment from branding patch ......................................................................
userportal,webadmin: fix comment from branding patch - Fixed BrandingServlet getFullPath according to branding patch comment (pass root in as parameter). - Fixed potential double forward slash. Change-Id: I9fad1cda014713724f294fe8454fb5f36870c137 Signed-off-by: Alexander Wels <[email protected]> --- M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java M backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java M frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java 4 files changed, 56 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/15702/1 diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java index a831365..a111e0d 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java @@ -179,5 +179,24 @@ return file; } - + /** + * Merge two paths into a single path. For instance base = xx/yy/ and path is /aa/bb returns + * xx/yy/aa/bb. + * @param base Base part of the path. + * @param path The path to attach to the base. + * @return The merger of base and path. + */ + public static String mergePaths(final String base, final String path) { + String mergedPath = "/"; + if (base != null) { + mergedPath = base; + // Make sure base always ends with a '/' + mergedPath += !base.endsWith("/") ? "/" : ""; + } + if (path != null) { + // Make sure path never begins with a '/' + mergedPath += path.startsWith("/") ? path.substring(1) : path; + } + return mergedPath; + } } diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java index 9507882..e6f5ec1 100644 --- a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java @@ -206,6 +206,32 @@ verify(responseOut).write((byte[]) anyObject(), eq(0), anyInt()); } + @Test + public void testMergePath() { + String mergedPath = ServletUtils.mergePaths("/xx/yy/", "/aa/bb"); + assertEquals("Path should be '/xx/yy/aa/bb'", "/xx/yy/aa/bb", mergedPath); + mergedPath = ServletUtils.mergePaths("/xx/yy", "aa/bb"); + assertEquals("Path should be '/xx/yy/aa/bb'", "/xx/yy/aa/bb", mergedPath); + mergedPath = ServletUtils.mergePaths("/xx/yy/", "aa/bb"); + assertEquals("Path should be '/xx/yy/aa/bb'", "/xx/yy/aa/bb", mergedPath); + mergedPath = ServletUtils.mergePaths("/xx/yy", "/aa/bb"); + assertEquals("Path should be '/xx/yy/aa/bb'", "/xx/yy/aa/bb", mergedPath); + mergedPath = ServletUtils.mergePaths("", "/aa/bb"); + assertEquals("Path should be '/aa/bb'", "/aa/bb", mergedPath); + mergedPath = ServletUtils.mergePaths(null, "/aa/bb"); + assertEquals("Path should be '/aa/bb'", "/aa/bb", mergedPath); + mergedPath = ServletUtils.mergePaths("", "aa/bb"); + assertEquals("Path should be '/aa/bb'", "/aa/bb", mergedPath); + mergedPath = ServletUtils.mergePaths(null, null); + assertEquals("Path should be '/'", "/", mergedPath); + mergedPath = ServletUtils.mergePaths("", ""); + assertEquals("Path should be '/'", "/", mergedPath); + mergedPath = ServletUtils.mergePaths("/xx/yy", null); + assertEquals("Path should be '/xx/yy/'", "/xx/yy/", mergedPath); + mergedPath = ServletUtils.mergePaths("/xx/yy", ""); + assertEquals("Path should be '/xx/yy/'", "/xx/yy/", mergedPath); + } + private File createTempPng() { try { File file = File.createTempFile("favicon", ".png"); diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java index 04ca6f1..e6a2040 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java @@ -49,7 +49,7 @@ String path = request.getPathInfo(); // Locate the requested file: - String fullPath = getFullPath(path); + String fullPath = getFullPath(brandingManager.getBrandingRootPath().getAbsolutePath(), path); if (fullPath != null) { File file = new File(fullPath); if (!file.exists() || !file.canRead() || file.isDirectory()) { @@ -97,16 +97,18 @@ /** * Translate the passed in path into a real file path so we can locate * the appropriate file. + * @param brandingRootPath The path to the root of the branding. * @param path The path to translate. * @return A full absolute path for the passed in path. */ - String getFullPath(final String path) { + String getFullPath(final String brandingRootPath, final String path) { String result = null; - if (path != null && ServletUtils.isSane(path)) { + String mergedPath = ServletUtils.mergePaths(brandingRootPath, path); + if (path != null && ServletUtils.isSane(mergedPath)) { // Return a result relative to the branding root path. - result = brandingManager.getBrandingRootPath().getAbsolutePath() + path; + result = mergedPath; } else { - log.error("The path \"" + path + "\" is not sane"); //$NON-NLS-1$ //$NON-NLS-2$ + log.error("The path \"" + mergedPath + "\" is not sane"); //$NON-NLS-1$ //$NON-NLS-2$ } return result; } diff --git a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java index 820e577..b34e7da 100644 --- a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java +++ b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java @@ -106,19 +106,19 @@ @Test public void testGetFullPath_nullParameter() { - String fullPath = testServlet.getFullPath(null); + String fullPath = testServlet.getFullPath(mockFile.getAbsolutePath(), null); assertNull("Path should be null", fullPath); //$NON-NLS-1$ } @Test public void testGetFullPath_NonSaneParameter() { - String fullPath = testServlet.getFullPath("../something"); //$NON-NLS-1$ + String fullPath = testServlet.getFullPath(mockFile.getAbsolutePath(), "../something"); //$NON-NLS-1$ assertNull("Path should be null", fullPath); //$NON-NLS-1$ } @Test public void testGetFullPath_SaneParameter() { - String fullPath = testServlet.getFullPath("/branding/test"); //$NON-NLS-1$ + String fullPath = testServlet.getFullPath(mockFile.getAbsolutePath(), "/branding/test"); //$NON-NLS-1$ assertNotNull("Path should not be null", fullPath); //$NON-NLS-1$ assertEquals("Path should be '/abs/test/branding/test'", //$NON-NLS-1$ "/abs/test/branding/test", fullPath); //$NON-NLS-1$ -- To view, visit http://gerrit.ovirt.org/15702 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9fad1cda014713724f294fe8454fb5f36870c137 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
