Fix url validation errors (#11835)

* Fix issue where href validation would incorrectly consider any URL with a fragment to be invalid

* Resolve issue where getFragment only matched to the last # encountered instead of the first

* Add extra checks for http urls to ensure the right number of forward slashes are used

* Update isValidProtocol to only consider the protocol as characters up to and including the colon

* Update changelogs

packages/format-library/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 1.2.3 (Unreleased)
+
+### Bug fixes
+- Link URL validation now works correctly when a URL includes a fragment. Previously any URL containing a fragment failed validation.
+- Link URL validation catches an incorrect number of forward slashes following a url using the http protocol.
+
 ## 1.2.2 (2018-11-15)
 
 ## 1.2.1 (2018-11-12)

packages/format-library/src/link/test/utils.js

@@ -26,6 +26,8 @@ describe( 'isValidHref', () => {
 			expect( isValidHref( 'https://test.com' ) ).toBe( true );
 			expect( isValidHref( 'http://test-with-hyphen.com' ) ).toBe( true );
 			expect( isValidHref( 'http://test.com/' ) ).toBe( true );
+			expect( isValidHref( 'http://test.com#fragment' ) ).toBe( true );
+			expect( isValidHref( 'http://test.com/path#fragment' ) ).toBe( true );
 			expect( isValidHref( 'http://test.com/with/path/separators' ) ).toBe( true );
 			expect( isValidHref( 'http://test.com/with?query=string&params' ) ).toBe( true );
 		} );
@@ -36,6 +38,7 @@ describe( 'isValidHref', () => {
 			expect( isValidHref( 'mailto: test@somewhere.com' ) ).toBe( false );
 			expect( isValidHref( 'ht#tp://this/is/invalid' ) ).toBe( false );
 			expect( isValidHref( 'ht#tp://th&is/is/invalid' ) ).toBe( false );
+			expect( isValidHref( 'http:/test.com' ) ).toBe( false );
 			expect( isValidHref( 'http://?test.com' ) ).toBe( false );
 			expect( isValidHref( 'http://#test.com' ) ).toBe( false );
 			expect( isValidHref( 'http://test.com?double?params' ) ).toBe( false );

packages/format-library/src/link/utils.js

@@ -37,13 +37,19 @@ export function isValidHref( href ) {
 		return false;
 	}
 
-	// Does the href start with something that looks like a url protocol?
+	// Does the href start with something that looks like a URL protocol?
 	if ( /^\S+:/.test( trimmedHref ) ) {
 		const protocol = getProtocol( trimmedHref );
 		if ( ! isValidProtocol( protocol ) ) {
 			return false;
 		}
 
+		// Add some extra checks for http(s) URIs, since these are the most common use-case.
+		// This ensures URIs with an http protocol have exactly two forward slashes following the protocol.
+		if ( startsWith( protocol, 'http' ) && ! /^https?:\/\/[^\/\s]/i.test( trimmedHref ) ) {
+			return false;
+		}
+
 		const authority = getAuthority( trimmedHref );
 		if ( ! isValidAuthority( authority ) ) {
 			return false;
@@ -60,7 +66,7 @@ export function isValidHref( href ) {
 		}
 
 		const fragment = getFragment( trimmedHref );
-		if ( fragment && ! isValidFragment( trimmedHref ) ) {
+		if ( fragment && ! isValidFragment( fragment ) ) {
 			return false;
 		}
 	}

packages/url/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 2.3.1 (Unreleased)
+
+### Bug fixes
+- The `isValidProtocol` function now correctly considers the protocol of the URL as only incoporating characters up to and including the colon (':').
+- `getFragment` is now greedier and matches fragments from the first occurence of the '#' symbol instead of the last.
+
 ## 2.3.0 (2018-11-12)
 
 ### New Features

packages/url/src/index.js

@@ -37,13 +37,13 @@ export function getProtocol( url ) {
  *
  * @param {string} protocol The url protocol.
  *
- * @return {boolean} True if the argument is a valid protocol (e.g. http://, tel:).
+ * @return {boolean} True if the argument is a valid protocol (e.g. http:, tel:).
  */
 export function isValidProtocol( protocol ) {
 	if ( ! protocol ) {
 		return false;
 	}
-	return /^[a-z\-.\+]+[0-9]*:(?:\/\/)?\/?$/i.test( protocol );
+	return /^[a-z\-.\+]+[0-9]*:$/i.test( protocol );
 }
 
 /**
@@ -138,7 +138,7 @@ export function isValidQueryString( queryString ) {
  * @return {?string} The fragment part of the URL.
  */
 export function getFragment( url ) {
-	const matches = /^\S+(#[^\s\?]*)/.exec( url );
+	const matches = /^\S+?(#[^\s\?]*)/.exec( url );
 	if ( matches ) {
 		return matches[ 1 ];
 	}

packages/url/src/test/index.test.js

@@ -78,9 +78,7 @@ describe( 'isValidProtocol', () => {
 		expect( isValidProtocol( 'tel:' ) ).toBe( true );
 		expect( isValidProtocol( 'http:' ) ).toBe( true );
 		expect( isValidProtocol( 'https:' ) ).toBe( true );
-		expect( isValidProtocol( 'http://' ) ).toBe( true );
-		expect( isValidProtocol( 'https://' ) ).toBe( true );
-		expect( isValidProtocol( 'file:///' ) ).toBe( true );
+		expect( isValidProtocol( 'file:' ) ).toBe( true );
 		expect( isValidProtocol( 'test.protocol:' ) ).toBe( true );
 		expect( isValidProtocol( 'test-protocol:' ) ).toBe( true );
 		expect( isValidProtocol( 'test+protocol:' ) ).toBe( true );