[JENKINS-62261] RepositoryBrowser: Use UrlValidator.ALLOW_LOCAL_URLS #890
Conversation
|
I agree that the validator should accept local URLs as valid, especially with the 2018 decision by ICANN to reserve |
| @@ -43,6 +43,18 @@ public void testInitialChecksOnRepoUrlWithVariable() throws Exception { | |||
| assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url), is(FormValidation.ok())); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testDomainLevelChecksOnRepoUrlAllowLocalHostnames() throws Exception { | |||
| String url = "https://localhost/space/git-plugin/git/source"; | |||
There was a problem hiding this comment.
I expect that most users running a localhost repository server are not using SSL to encrypt the localhost traffic. If we're acting like this is a valid use case that should not generate a warning message, then let's accept http instead of https.
| String url = "https://localhost/space/git-plugin/git/source"; | |
| String url = "http://localhost/space/git-plugin/git/source"; |
I think this is not a valid use case because Jenkins itself will display a warning to the user if they set the Jenkins URL to be localhost. The user is still allowed to ignore the error text and apply the change. However, I'm open to allowing this case if the UrlValidator allows it. As far as I can tell from the automated test, the UrlValidator does not allow it.
|
@sratz I took the concepts you were using and extended them further in the AssemblaWeb checks. I've pushed them to this branch so that you can see them and comment. It will be a while before I can spend the time to make the same changes in the main implementation. Let me know what you think of the compromises and changes that I've made. I'm intentionally using the Java URL class constructor to detect certain classes of issues, then specifically skipping the Apache UrlValidator() call if the hostname contains one of the known local network suffixes. It would have been much nicer if UrlValidator() recognized those hostname suffixes as local hostnames, but I believe that decision came after the version of UrlValidator was released. |
|
The changes look good. I noticed that there is an additional API we could potentially use to solve this: However, this is a static method and can only be called before anyone calls |
The URL to the git repository browser must be a URL rather than a URI. Collect better exception messages from the URL constructor than are available from the URI constructor.
|
See #940 for alternatives in the new release of Apache commons validator. |
|
@sratz @MarkEWaite I have used the DomainValidator API to solve this problem, please have a look at #967 |
|
Thanks a lot @rishabhBudhouliya! |
JENKINS-62261 - Allow local URLs in repo browsers
Relax URL validation in repository browser such that *.corp domains and local hostnames are not dismissed as invalid.
Checklist
Types of changes