diff --git a/packages/url/CHANGELOG.md b/packages/url/CHANGELOG.md index 2e29586280a67..2845b6c35e4ac 100644 --- a/packages/url/CHANGELOG.md +++ b/packages/url/CHANGELOG.md @@ -1,5 +1,9 @@ ## Master +### Bug Fixes + +- `getQueryString` now correctly considers hash fragments when considering whether to return a query string. Previously, `getQueryString( 'https://example.com/#?foo' )` would wrongly return `'foo'` as its result. A hash fragment is always the last segment of a URL, and the querystring must always precede it ([see reference specification](https://url.spec.whatwg.org/#absolute-url-with-fragment-string)). + ## 2.11.0 (2020-02-10) ### Bug Fixes diff --git a/packages/url/README.md b/packages/url/README.md index b08f57b7e3959..71b2a0d47a5ab 100644 --- a/packages/url/README.md +++ b/packages/url/README.md @@ -178,8 +178,7 @@ Returns the query string part of the URL. _Usage_ ```js -const queryString1 = getQueryString( 'http://localhost:8080/this/is/a/test?query=true#fragment' ); // 'query=true' -const queryString2 = getQueryString( 'https://wordpress.org#fragment?query=false&search=hello' ); // 'query=false&search=hello' +const queryString = getQueryString( 'http://localhost:8080/this/is/a/test?query=true#fragment' ); // 'query=true' ``` _Parameters_ diff --git a/packages/url/src/get-query-string.js b/packages/url/src/get-query-string.js index 8341da1f9fa1b..e1437f4e78a13 100644 --- a/packages/url/src/get-query-string.js +++ b/packages/url/src/get-query-string.js @@ -5,15 +5,18 @@ * * @example * ```js - * const queryString1 = getQueryString( 'http://localhost:8080/this/is/a/test?query=true#fragment' ); // 'query=true' - * const queryString2 = getQueryString( 'https://wordpress.org#fragment?query=false&search=hello' ); // 'query=false&search=hello' + * const queryString = getQueryString( 'http://localhost:8080/this/is/a/test?query=true#fragment' ); // 'query=true' * ``` * * @return {string|void} The query string part of the URL. */ export function getQueryString( url ) { - const matches = /^\S+?\?([^\s#]+)/.exec( url ); - if ( matches ) { - return matches[ 1 ]; + let query; + try { + query = new URL( url ).search.substring( 1 ); + } catch ( error ) {} + + if ( query ) { + return query; } } diff --git a/packages/url/src/test/index.test.js b/packages/url/src/test/index.test.js index d14572627de14..383575dc892b6 100644 --- a/packages/url/src/test/index.test.js +++ b/packages/url/src/test/index.test.js @@ -272,11 +272,6 @@ describe( 'isValidPath', () => { describe( 'getQueryString', () => { it( 'returns the query string of a URL', () => { - expect( - getQueryString( - 'https://user:password@www.test-this.com:1020/test-path/file.extension#anchor?query=params&more' - ) - ).toBe( 'query=params&more' ); expect( getQueryString( 'http://user:password@www.test-this.com:1020/test-path/file.extension?query=params&more#anchor' @@ -305,16 +300,21 @@ describe( 'getQueryString', () => { 'https://andalouses.example/beach?foo[]=bar&foo[]=baz' ) ).toBe( 'foo[]=bar&foo[]=baz' ); - expect( getQueryString( 'test.com?foo[]=bar&foo[]=baz' ) ).toBe( + expect( getQueryString( 'https://test.com?foo[]=bar&foo[]=baz' ) ).toBe( 'foo[]=bar&foo[]=baz' ); - expect( getQueryString( 'test.com?foo=bar&foo=baz?test' ) ).toBe( - 'foo=bar&foo=baz?test' - ); + expect( + getQueryString( 'https://test.com?foo=bar&foo=baz?test' ) + ).toBe( 'foo=bar&foo=baz?test' ); } ); it( 'returns undefined when the provided does not contain a url query string', () => { expect( getQueryString( '' ) ).toBeUndefined(); + expect( + getQueryString( + 'https://user:password@www.test-this.com:1020/test-path/file.extension#anchor?query=params&more' + ) + ).toBeUndefined(); expect( getQueryString( 'https://wordpress.org/test-path#anchor' ) ).toBeUndefined(); @@ -326,11 +326,10 @@ describe( 'getQueryString', () => { ).toBeUndefined(); expect( getQueryString( 'https://wordpress.org/' ) ).toBeUndefined(); expect( getQueryString( 'https://localhost:8080' ) ).toBeUndefined(); - expect( getQueryString( 'https://' ) ).toBeUndefined(); - expect( getQueryString( 'https:///test' ) ).toBeUndefined(); - expect( getQueryString( 'https://#' ) ).toBeUndefined(); - expect( getQueryString( 'https://?' ) ).toBeUndefined(); - expect( getQueryString( 'test.com' ) ).toBeUndefined(); + expect( getQueryString( 'invalid' ) ).toBeUndefined(); + expect( + getQueryString( 'https://example.com/empty?' ) + ).toBeUndefined(); } ); } );