Lior Vernia has uploaded a new change for review. Change subject: webadmin: Added validation to ProviderModel ......................................................................
webadmin: Added validation to ProviderModel Making sure that the provider name is not empty, and that its URL is approximately fine. Change-Id: I754a7ce4e2aa719b0284d2dc0021822af2629b07 Signed-off-by: Lior Vernia <[email protected]> --- A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Uri.java A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/UriAuthority.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/UrlValidation.java A frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/UriTest.java A frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/UrlValidationTest.java M frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java 7 files changed, 319 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/95/14695/1 diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Uri.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Uri.java new file mode 100644 index 0000000..8727b94 --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Uri.java @@ -0,0 +1,79 @@ +package org.ovirt.engine.ui.uicommonweb; + +import com.google.gwt.regexp.shared.MatchResult; +import com.google.gwt.regexp.shared.RegExp; + +/** + * This is a crude implementation of a URI for our current needs. It does NOT comply with the definition of RFC 3986; + * generally speaking it is not as picky about the characters it accepts, and its partition into components is not as + * fine. It should be extended as better parsing is needed.<br> + * <br> + * Usage: The constructor is to be passed the candidate URI as argument. Before any URI component is accessed, the URI + * should be checked for validity. In case an optional capturing group wasn't matched, its getter will return an empty + * String. The return value of the getters is undefined if the URI is invalid, and might even be null. + */ +public class Uri { + + public static final String SCHEME_HTTP = "http"; //$NON-NLS-1$ + + private static final RegExp PATTERN_URI = + RegExp.compile("^(?:(.*)://)?([^/]*)(/.*)?$", "i"); //$NON-NLS-1$ $NON-NLS-2$ + + private boolean valid; + private String scheme; + private UriAuthority authority; + private String path; + + public Uri(String uri) { + MatchResult matcher = PATTERN_URI.exec(uri == null ? new String() : uri); + valid = matcher != null; + if (valid) { + setScheme(matcher.getGroup(1)); + setAuthority(new UriAuthority(matcher.getGroup(2))); + setPath(matcher.getGroup(3)); + } + } + + @Override + public String toString() { + String uri = new String(); + if (!scheme.isEmpty()) { + uri += scheme + "://"; //$NON-NLS-1$ + } + uri += authority.toString(); + uri += path; + return uri; + } + + public boolean isValid() { + return valid; + } + + public String getScheme() { + return scheme; + } + + public void setScheme(String scheme) { + this.scheme = (scheme == null) ? new String() : scheme; + } + + public UriAuthority getAuthority() { + return authority; + } + + public void setAuthority(UriAuthority authority) { + this.authority = authority; + if (!authority.isValid()) { + valid = false; + } + } + + public String getPath() { + return path; + } + + public void setPath(String path) { + this.path = (path == null) ? new String() : path; + } + +} diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/UriAuthority.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/UriAuthority.java new file mode 100644 index 0000000..67d0d3f --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/UriAuthority.java @@ -0,0 +1,53 @@ +package org.ovirt.engine.ui.uicommonweb; + +import com.google.gwt.regexp.shared.MatchResult; +import com.google.gwt.regexp.shared.RegExp; + +public class UriAuthority { + + private static final RegExp PATTERN_AUTHORITY = RegExp.compile("^([^:]*)(?::(.*))?$", "i"); //$NON-NLS-1$ $NON-NLS-2$ + + private boolean valid; + private String host; + private String port; + + public UriAuthority(String authority) { + MatchResult matcher = PATTERN_AUTHORITY.exec(authority == null ? new String() : authority); + valid = matcher != null; + if (valid) { + setHost(matcher.getGroup(1)); + setPort(matcher.getGroup(2)); + } + } + + @Override + public String toString() { + String authority = new String(); + authority += host; + if (!port.isEmpty()) { + authority += ':' + port; + } + return authority; + } + + public boolean isValid() { + return valid; + } + + public String getHost() { + return host; + } + + public void setHost(String host) { + this.host = (host == null) ? new String() : host; + } + + public String getPort() { + return port; + } + + public void setPort(String port) { + this.port = (port == null) ? new String() : port; + } + +} diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java index fbbc924..169f14d 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java @@ -6,9 +6,13 @@ import org.ovirt.engine.core.compat.StringHelper; import org.ovirt.engine.ui.frontend.Frontend; import org.ovirt.engine.ui.uicommonweb.UICommand; +import org.ovirt.engine.ui.uicommonweb.Uri; import org.ovirt.engine.ui.uicommonweb.models.EntityModel; import org.ovirt.engine.ui.uicommonweb.models.Model; import org.ovirt.engine.ui.uicommonweb.models.SearchableListModel; +import org.ovirt.engine.ui.uicommonweb.validation.IValidation; +import org.ovirt.engine.ui.uicommonweb.validation.NotEmptyValidation; +import org.ovirt.engine.ui.uicommonweb.validation.UrlValidation; import org.ovirt.engine.ui.uicompat.ConstantsManager; import org.ovirt.engine.ui.uicompat.FrontendActionAsyncResult; import org.ovirt.engine.ui.uicompat.IFrontendActionAsyncCallback; @@ -71,7 +75,16 @@ } private boolean Validate() { - return true; + getName().ValidateEntity(new IValidation[] { new NotEmptyValidation() }); + + Uri url = new Uri((String) getUrl().getEntity()); + if (url.getScheme().isEmpty()) { + url.setScheme(Uri.SCHEME_HTTP); + getUrl().setEntity(url.toString()); + } + getUrl().ValidateEntity(new IValidation[] { new NotEmptyValidation(), new UrlValidation() }); + + return getName().getIsValid() && getUrl().getIsValid(); } private void cancel() { diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/UrlValidation.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/UrlValidation.java new file mode 100644 index 0000000..ddc41ea --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/UrlValidation.java @@ -0,0 +1,39 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +import org.ovirt.engine.ui.uicommonweb.Uri; +import org.ovirt.engine.ui.uicompat.ConstantsManager; + +public class UrlValidation implements IValidation { + + @Override + public ValidationResult validate(Object value) { + Uri uri = new Uri((String) value); + ValidationResult res = new ValidationResult(); + + if (!uri.isValid()) { + res.setSuccess(false); + res.getReasons().add(getUriMessage()); + return res; + } + + res = getHostValidation().validate(uri.getAuthority().getHost()); + if (!uri.getScheme().equalsIgnoreCase(Uri.SCHEME_HTTP)) { + res.setSuccess(false); + res.getReasons().add(getSchemeMessage()); + } + return res; + } + + protected String getUriMessage() { + return ConstantsManager.getInstance().getConstants().uriInvalidFormat(); + } + + protected String getSchemeMessage() { + return ConstantsManager.getInstance().getConstants().urlSchemeNotHttp(); + } + + protected HostAddressValidation getHostValidation() { + return new HostAddressValidation(); + } + +} diff --git a/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/UriTest.java b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/UriTest.java new file mode 100644 index 0000000..5a52201 --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/UriTest.java @@ -0,0 +1,54 @@ +package org.ovirt.engine.ui.uicommonweb; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.Collection; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class UriTest { + + private String uriCandidate; + + public UriTest(String uriCandidate) { + this.uriCandidate = uriCandidate; + } + + @Test + public void runTest() { + Uri uri = new Uri(uriCandidate); + assertTrue(uri.isValid()); + assertEquals(uriCandidate.toLowerCase(), uri.toString()); + } + + @Parameterized.Parameters + public static Collection<Object[]> comparisonParameters() { + return Arrays.asList(new Object[][] { + { "www.redhat.com" }, //$NON-NLS-1$ + { "www.redhat.com/" }, //$NON-NLS-1$ + { "www.redhat.com/main" }, //$NON-NLS-1$ + { "www.redhat.com/main/" }, //$NON-NLS-1$ + { "www.redhat.com/main/index.html" }, //$NON-NLS-1$ + { "http://www.redhat.com"; }, //$NON-NLS-1$ + { "http://www.redhat.com/"; }, //$NON-NLS-1$ + { "http://www.redhat.com/main"; }, //$NON-NLS-1$ + { "http://www.redhat.com/main/"; }, //$NON-NLS-1$ + { "http://www.redhat.com/main/index.html"; }, //$NON-NLS-1$ + { "www.redhat.com:80" }, //$NON-NLS-1$ + { "www.redhat.com:80/" }, //$NON-NLS-1$ + { "www.redhat.com:80/main" }, //$NON-NLS-1$ + { "www.redhat.com:80/main/" }, //$NON-NLS-1$ + { "www.redhat.com:80/main/index.html" }, //$NON-NLS-1$ + { "http://www.redhat.com:80"; }, //$NON-NLS-1$ + { "http://www.redhat.com:80/"; }, //$NON-NLS-1$ + { "http://www.redhat.com:80/main"; }, //$NON-NLS-1$ + { "http://www.redhat.com:80/main/"; }, //$NON-NLS-1$ + { "http://www.redhat.com:80/main/index.html"; } //$NON-NLS-1$ + }); + } +} diff --git a/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/UrlValidationTest.java b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/UrlValidationTest.java new file mode 100644 index 0000000..ad7abc5 --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/UrlValidationTest.java @@ -0,0 +1,73 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; + +import java.util.Arrays; +import java.util.Collection; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.mockito.Spy; + +@RunWith(Parameterized.class) +public class UrlValidationTest { + + @Spy + private UrlValidation urlValidation; + + private class TestableHostValidation extends HostAddressValidation { + @Override + protected String composeMessage() { + return null; + } + } + + private TestableHostValidation hostValidation; + + private String url; + private boolean expectedResult; + + public UrlValidationTest(String url, boolean expectedResult) { + this.url = url; + this.expectedResult = expectedResult; + } + + @Before + public void setup() { + urlValidation = spy(new UrlValidation()); + hostValidation = new TestableHostValidation(); + doReturn(null).when(urlValidation).getUriMessage(); + doReturn(null).when(urlValidation).getSchemeMessage(); + doReturn(hostValidation).when(urlValidation).getHostValidation(); + } + + @Test + public void runTest() { + assertEquals(expectedResult, urlValidation.validate(url).getSuccess()); + } + + @Parameterized.Parameters + public static Collection<Object[]> comparisonParameters() { + return Arrays.asList(new Object[][] { + { null, false }, + { "", false }, //$NON-NLS-1$ + { "http://";, false }, //$NON-NLS-1$ + { "www.redhat.com", false }, //$NON-NLS-1$ + { "192.168.0.1", false }, //$NON-NLS-1$ + { "ftp://www.redhat.com";, false }, //$NON-NLS-1$ + { "ftp://192.168.0.1";, false }, //$NON-NLS-1$ + + { "http://www.redhat.com";, true }, //$NON-NLS-1$ + { "http://www.redhat.com/main";, true }, //$NON-NLS-1$ + { "http://www.redhat.com/main/index.html";, true }, //$NON-NLS-1$ + { "http://www.redhat.com:80";, true }, //$NON-NLS-1$ + { "http://www.redhat.com:80/main";, true }, //$NON-NLS-1$ + { "http://www.redhat.com:80/main/index.html";, true } //$NON-NLS-1$ + }); + } + +} diff --git a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java index bc3e49c..b8b7a0a 100644 --- a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java +++ b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java @@ -964,9 +964,15 @@ @DefaultStringValue("Invalid E-Mail address") String invalidEmailAddressInvalidReason(); - @DefaultStringValue("Address is not a valid host name or IP address.") + @DefaultStringValue("The given host address is neither a valid host name nor a valid IP address.") String addressIsNotValidHostNameOrIpAddressInvalidReason(); + @DefaultStringValue("Given URI is of an invalid format.") + String uriInvalidFormat(); + + @DefaultStringValue("Given URL contains invalid scheme, only 'http://' is allowed.") + String urlSchemeNotHttp(); + @DefaultStringValue("Switch to maintenance mode to enable Upgrade.") String switchToMaintenanceModeToEnableUpgradeReason(); -- To view, visit http://gerrit.ovirt.org/14695 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I754a7ce4e2aa719b0284d2dc0021822af2629b07 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
