Alexander Wels has posted comments on this change. Change subject: userportal,webadmin: fix comment from branding patch ......................................................................
Patch Set 3: (2 inline comments) base/BRANDING_PACKAGE/path base is determined at runtime (in our case it is the etc path) BRANDING_PACKAGE/path combination is the request path to the servlet.So if the browser requests: http://localhost/webadmin/00-ovirt.brand/images/logo.png the path parameter will be /00-ovirt.brand/images/logo.png and the base will be etcDir + 'branding' so we end up with the absolute path of etcDir/branding/00-ovirt.brand/images/logo.png as the file. 'branding' is added by the BrandingManager as part of the path and really is the only hardcoded assumption about the path to the resource. .................................................... File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java Line 101: * @param brandingRootPath The path to the root of the branding. Cannot be null Line 102: * @param path The path to translate. Line 103: * @return A full absolute path for the passed in path. Line 104: */ Line 105: String getFullPath(final File brandingRootPath, final String path) { Because path comes from the HttpRequest and it is a string. It makes no sense to convert it to a File, and then back to string again when I pass it to getPath Line 106: String result = null; Line 107: String mergedPath = FileSystems.getDefault().getPath(brandingRootPath.getAbsolutePath(), Line 108: path == null ? "": path).toString(); Line 109: if (path != null && ServletUtils.isSane(mergedPath)) { Line 105: String getFullPath(final File brandingRootPath, final String path) { Line 106: String result = null; Line 107: String mergedPath = FileSystems.getDefault().getPath(brandingRootPath.getAbsolutePath(), Line 108: path == null ? "": path).toString(); Line 109: if (path != null && ServletUtils.isSane(mergedPath)) { The isSane method is used all over the place to determine if the path is sane. I did not want to reinvent the wheel even if the current wheel is square right now. Line 110: // Return a result relative to the branding root path. Line 111: result = mergedPath; Line 112: } else { Line 113: log.error("The path \"" + mergedPath + "\" is not sane"); //$NON-NLS-1$ //$NON-NLS-2$ -- To view, visit http://gerrit.ovirt.org/15702 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9fad1cda014713724f294fe8454fb5f36870c137 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
