Repository: ambari Updated Branches: refs/heads/trunk 21bf7c330 -> 621303369
AMBARI-5042 Ambari Repo URL validator rejecting valid yum repo file:/// URL. (jaoki) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/62130336 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/62130336 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/62130336 Branch: refs/heads/trunk Commit: 62130336971b2a8c6f84557b3931eaa48c2b4b4b Parents: 21bf7c3 Author: Jun Aoki <ja...@apache.org> Authored: Fri Feb 6 10:22:04 2015 -0800 Committer: Jun Aoki <ja...@apache.org> Committed: Fri Feb 6 10:22:04 2015 -0800 ---------------------------------------------------------------------- .../AmbariManagementControllerImpl.java | 41 ++++++++++----- .../AmbariManagementControllerImplTest.java | 53 ++++++++++++++++++++ 2 files changed, 81 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/62130336/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java index 7d78a4a..7e811e2 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java @@ -3074,26 +3074,41 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle String[] suffixes = configs.getRepoValidationSuffixes(request.getOsType()); for (String suffix : suffixes) { String formatted_suffix = String.format(suffix, repoName); - String spec = request.getBaseUrl(); + String spec = request.getBaseUrl().trim(); + // This logic is to identify if the end of baseurl has a slash ('/') and/or the beginning of suffix String (e.g. "/repodata/repomd.xml") + // has a slash and they can form a good url. + // e.g. "http://baseurl.com/"; + "/repodata/repomd.xml" becomes "http://baseurl.com/repodata/repomd.xml"; but not "http://baseurl.com//repodata/repomd.xml"; if (spec.charAt(spec.length() - 1) != '/' && formatted_suffix.charAt(0) != '/') { - spec = request.getBaseUrl() + "/" + formatted_suffix; + spec = spec + "/" + formatted_suffix; } else if (spec.charAt(spec.length() - 1) == '/' && formatted_suffix.charAt(0) == '/') { - spec = request.getBaseUrl() + formatted_suffix.substring(1); + spec = spec + formatted_suffix.substring(1); } else { - spec = request.getBaseUrl() + formatted_suffix; + spec = spec + formatted_suffix; } - try { - IOUtils.readLines(usp.readFrom(spec)); - } catch (IOException ioe) { - errorMessage = "Could not access base url . " + request.getBaseUrl() + " . "; - if (LOG.isDebugEnabled()) { - errorMessage += ioe; - } else { - errorMessage += ioe.getMessage(); + // if spec contains "file://" then check local file system. + final String FILE_SCHEME = "file://"; + if(spec.toLowerCase().startsWith(FILE_SCHEME)){ + String filePath = spec.substring(FILE_SCHEME.length()); + File f = new File(filePath); + if(!f.exists()){ + errorMessage = "Could not access base url . " + spec + " . "; + break; + } + + }else{ + try { + IOUtils.readLines(usp.readFrom(spec)); + } catch (IOException ioe) { + errorMessage = "Could not access base url . " + request.getBaseUrl() + " . "; + if (LOG.isDebugEnabled()) { + errorMessage += ioe; + } else { + errorMessage += ioe.getMessage(); + } + break; } - break; } } http://git-wip-us.apache.org/repos/asf/ambari/blob/62130336/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java index 6aee03b..208218c 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java @@ -48,6 +48,7 @@ import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.ComponentInfo; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.MaintenanceState; +import org.apache.ambari.server.state.RepositoryInfo; import org.apache.ambari.server.state.SecurityType; import org.apache.ambari.server.state.Service; import org.apache.ambari.server.state.ServiceComponent; @@ -1817,4 +1818,56 @@ public class AmbariManagementControllerImplTest { } } + + @Test + public void testVerifyRepositires() throws Exception { + // member state mocks + Injector injector = createStrictMock(Injector.class); + Capture<AmbariManagementController> controllerCapture = new Capture<AmbariManagementController>(); + + // expectations + // constructor init + injector.injectMembers(capture(controllerCapture)); + expect(injector.getInstance(Gson.class)).andReturn(null); + expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(null); + expect(injector.getInstance(KerberosHelper.class)).andReturn(createNiceMock(KerberosHelper.class)); + + RepositoryInfo dummyRepoInfo = new RepositoryInfo(); + dummyRepoInfo.setRepoName("repo_name"); + + expect(ambariMetaInfo.getRepository("stackName", "stackVersion", "redhat6", "repoId")).andReturn(dummyRepoInfo); + + Configuration configuration = createNiceMock(Configuration.class); + String[] suffices = {"/repodata/repomd.xml"}; + expect(configuration.getRepoValidationSuffixes("redhat6")).andReturn(suffices); + + // replay mocks + replay(injector, clusters, ambariMetaInfo, configuration); + + // test + AmbariManagementController controller = new AmbariManagementControllerImpl(null, clusters, injector); + setAmbariMetaInfo(ambariMetaInfo, controller); + + // Manually injected + Class<?> c = controller.getClass(); + Field f = c.getDeclaredField("configs"); + f.setAccessible(true); + f.set(controller, configuration); + + Set<RepositoryRequest> requests = new HashSet<RepositoryRequest>(); + RepositoryRequest request = new RepositoryRequest("stackName", "stackVersion", "redhat6", "repoId"); + request.setBaseUrl("file:///some/repo"); + requests.add(request); + + // A wrong file path is passed and IllegalArgumentException is expected + try{ + controller.verifyRepositories(requests); + Assert.fail("IllegalArgumentException is expected"); + }catch(IllegalArgumentException e){ + Assert.assertEquals("Could not access base url . file:///some/repo/repodata/repomd.xml . ", e.getMessage()); + } + + verify(injector, clusters, ambariMetaInfo, configuration); + } + }
