Alon Bar-Lev has posted comments on this change.
Change subject: userportal,webadmin: fix comment from branding patch
......................................................................
Patch Set 3: (2 inline comments)
I will give you an example why this may break.
If we have two search paths instead of one:
/etc/ovirt-engine/branding
/usr/share/ovirt-engine/branding
Then you will not be able to use the notation as you do not have the
information of which package it is...
Or... if at one day we will have the branding package location within
configuration, then you will not have base at all.
The osinfo packages will behaves in similar manner, maybe we have few of these.
At the end I would like to sync all plugin methods to behave the same... less
assumption on underline implementation the better.
But as I wrote it is not that important right now, as the interface is not
effected.
....................................................
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) {
I would have put the conversion closest to the faulted component.
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)) {
Well, this should be fixed too.
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
