Skip to content

Commit

Permalink
Merge pull request #802 from wheresrhys/rhys/url-normalization
Browse files Browse the repository at this point in the history
Rhys/url normalization
  • Loading branch information
wheresrhys authored Aug 13, 2024
2 parents bf2a29c + 3ac2a28 commit d01d7a9
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 31 deletions.
9 changes: 8 additions & 1 deletion docs/docs/@fetch-mock/core/route/matcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,14 @@ URL matchers and function matchers can be passed in as standalone values. They c

All of the following can be passed in directly as the `matcher` argument or as the property `{url: ...}` when combining with other matchers or options.

> Note that if using a Full url matcher or `end:`, fetch-mock ([for good reason](https://url.spec.whatwg.org/#url-equivalence)) is unable to distinguish whether URLs without a path end in a trailing slash or not i.e. `http://thing` is treated the same as `http://thing/`
#### URL matching oddities

`fetch` and other standards have strong opinions about how to interpret URLs. `fetch-mock` attempts to respect these, which can lead to unexpected behaviour. The notes below apply to all types of url matcher.

1. Trailing slashes are ignored i.e. `http://thing` is treated the same as `http://thing/` ([read the spec](https://url.spec.whatwg.org/#url-equivalence))
2. When using dot segments in urls `fetch-mock` will match both the full path containing dot segments, and the path ot resolves to e.g. `/path/../other-path` will match `/path/../other-path` and `/other-path`
3. `fetch` will convert any protocol-less urls to ones using the protocol of the current page e.g. if the browser is at `**http:**//a.com` and your application calls `fetch('//some.url')`, a request will be made to `**http:**//some.url`. However, to discourage writing tests that pass in one environment but not another, `fetch-mock` **will only** match requests where the protocol (or lack of) is exactly the same as the route. e.g. `begin://a.com` will match `//a.com/path` but not `http://a.com/path`
4. Fetches for urls relative to the current page e.g. `fetch('image.jpg)` are currently poorly supported and should be avoided. [This issue](https://github.com/wheresrhys/fetch-mock/issues/763) contains a proposal of how to deal with them.

### Full url

Expand Down
11 changes: 7 additions & 4 deletions packages/core/src/Matchers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,13 @@ const stringMatchers: { [key: string]: UrlMatcherGenerator } = {
return true;
};
},
path:
(targetString) =>
({ url }) =>
getPath(url) === targetString,
path: (targetString) => {
const dotlessTargetString = getPath(targetString);
return ({ url }) => {
const path = getPath(url)
return path === targetString || path === dotlessTargetString;
}
}
};
const getHeaderMatcher: MatcherGenerator = ({ headers: expectedHeaders }) => {
if (!expectedHeaders) {
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/RequestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ interface DerivedRequestOptions {

export type NormalizedRequestOptions = RequestInit | (RequestInit & DerivedRequestOptions);

export function hasCredentialsInUrl (url: string): boolean {
const urlObject = new URL(url, protocolRelativeUrlRX.test(url) ? 'http://dummy' : undefined);
return Boolean(urlObject.username || urlObject.password);
}

export function normalizeUrl(url: string | String | URL) {
if (url instanceof URL) {
Expand All @@ -23,7 +27,7 @@ export function normalizeUrl(url: string | String | URL) {
return new URL(primitiveUrl).href;
}
if (protocolRelativeUrlRX.test(primitiveUrl)) {
return new URL(primitiveUrl, 'http://dummy').href;
return new URL(primitiveUrl, 'http://dummy').href.replace(/^[a-z]+:/, '');
}
const urlInstance = new URL(primitiveUrl, 'http://dummy');
return urlInstance.pathname + urlInstance.search;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/Router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Route, { UserRouteConfig, RouteResponsePromise, RouteConfig, RouteRespons
import { isUrlMatcher, isFunctionMatcher } from './Matchers.js';
import { RouteMatcher } from './Matchers.js';
import { FetchMockConfig } from './FetchMock.js';
import { hasCredentialsInUrl } from './RequestUtils.js';
import type { CallLog } from './CallHistory.js';

export type ResponseConfigProp = "body" | "headers" | "throws" | "status" | "redirectUrl";
Expand Down Expand Up @@ -74,8 +75,7 @@ function throwSpecExceptions({ url, options: { headers, method, body } }: CallLo
}
});
}
const urlObject = new URL(url);
if (urlObject.username || urlObject.password) {
if (hasCredentialsInUrl(url)) {
throw new TypeError(
`Request cannot be constructed from a URL that includes credentials: ${url}`,
);
Expand Down
98 changes: 75 additions & 23 deletions packages/core/src/__tests__/Matchers/url.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ describe('url matching', () => {
expect(route.matcher({ url: 'http://a.com/paths' })).toBe(false);
expect(route.matcher({ url: 'http://a.co/path' })).toBe(false);
expect(route.matcher({ url: 'http://a.com/path' })).toBe(true);
expect(route.matcher({ url: '//a.com/path' })).toBe(true);
});

it('match string objects', () => {
Expand Down Expand Up @@ -100,17 +99,6 @@ describe('url matching', () => {
expect(route.matcher({ url: '/a.com/' })).toBe(true);
});

it('match relative urls with dots', () => {
const route = new Route({ url: '/it.at/there/', response: 200 });
expect(route.matcher({ url: '/it.at/not/../there/' })).toBe(true);
expect(route.matcher({ url: './it.at/there/' })).toBe(true);
});

it('match absolute urls with dots', () => {
const route = new Route({ url: 'http://it.at/there/', response: 200 });
expect(route.matcher({ url: 'http://it.at/not/../there/' })).toBe(true);
});

it('match with multiple url patterns at once', () => {
const route = new Route({
url: {
Expand All @@ -124,18 +112,7 @@ describe('url matching', () => {
expect(route.matcher({ url: 'http://a.com/jar/of/jam' })).toBe(true);
});

describe('host normalisation', () => {
it('match exact pathless urls regardless of trailing slash', () => {
const route = new Route({ url: 'http://a.com/', response: 200 });

expect(route.matcher({ url: 'http://a.com/' })).toBe(true);
expect(route.matcher({ url: 'http://a.com' })).toBe(true);

const route2 = new Route({ url: 'http://b.com', response: 200 });
expect(route2.matcher({ url: 'http://b.com/' })).toBe(true);
expect(route2.matcher({ url: 'http://b.com' })).toBe(true);
});
});

describe('data: URLs', () => {
it('match exact strings', () => {
Expand Down Expand Up @@ -200,4 +177,79 @@ describe('url matching', () => {
expect(route.matcher({ url: 'data:text/plain,12345' })).toBe(true);
});
});

describe('url resolution and normalisation', () => {
describe('trailing slashes', () => {
it('match exact pathless urls regardless of trailing slash', () => {
const route = new Route({ url: 'http://a.com/', response: 200 });

expect(route.matcher({ url: 'http://a.com/' })).toBe(true);
expect(route.matcher({ url: 'http://a.com' })).toBe(true);

const route2 = new Route({ url: 'http://b.com', response: 200 });
expect(route2.matcher({ url: 'http://b.com/' })).toBe(true);
expect(route2.matcher({ url: 'http://b.com' })).toBe(true);
});
});

describe('protocol agnostic urls', () => {
it('protocol agnostic url matches protocol agnostic url', () => {
const route = new Route({ url: '//a.com', response: 200 });
expect(route.matcher({ url: '//a.com' })).toBe(true);
});
it('protocol agnostic url does not match protocol specific url', () => {
const route = new Route({ url: '//a.com', response: 200 });
expect(route.matcher({ url: 'http://a.com' })).toBe(false);
});
it('protocol specific url does not match protocol agnostic url', () => {
const route = new Route({ url: 'http://a.com', response: 200 });
expect(route.matcher({ url: '//a.com' })).toBe(false);
});
it('protocol agnostic url matches beginning of protocol agnostic url', () => {
const route = new Route({ url: 'begin://a.com', response: 200 });
expect(route.matcher({ url: '//a.com/path' })).toBe(true);
});
it('protocol agnostic url does not match beginning of protocol specific url', () => {
const route = new Route({ url: 'begin://a.com', response: 200 });
expect(route.matcher({ url: 'http://a.com/path' })).toBe(false);
});
it('protocol specific url does not match beginning of protocol agnostic url', () => {
const route = new Route({ url: 'begin:http://a.com', response: 200 });
expect(route.matcher({ url: '//a.com/path' })).toBe(false);
});
})
describe('dot segments', () => {
it('dot segmented url matches dot segmented url', () => {
const relativeRoute = new Route({ url: '/it.at/not/../there', response: 200 });
expect(relativeRoute.matcher({ url: '/it.at/not/../there' })).toBe(true);
const absoluteRoute = new Route({ url: 'http://it.at/not/../there', response: 200 });
expect(absoluteRoute.matcher({ url: 'http:///it.at/not/../there' })).toBe(true);
});
it('dot segmented url matches dot segmentless url', () => {
const relativeRoute = new Route({ url: '/it.at/not/../there', response: 200 });
expect(relativeRoute.matcher({ url: '/it.at/there' })).toBe(true);
const absoluteRoute = new Route({ url: 'http://it.at/not/../there', response: 200 });
expect(absoluteRoute.matcher({ url: 'http:///it.at/there' })).toBe(true);
});
it('dot segmentless url matches dot segmented url', () => {
const relativeRoute = new Route({ url: '/it.at/there', response: 200 });
expect(relativeRoute.matcher({ url: '/it.at/not/../there' })).toBe(true);
const absoluteRoute = new Route({ url: 'http://it.at/there', response: 200 });
expect(absoluteRoute.matcher({ url: 'http:///it.at/not/../there' })).toBe(true);
});
it('dot segmented path matches dot segmented path', () => {
const relativeRoute = new Route({ url: 'path:/it.at/not/../there', response: 200 });
expect(relativeRoute.matcher({ url: '/it.at/not/../there' })).toBe(true);
});
it('dot segmented path matches dot segmentless path', () => {
const relativeRoute = new Route({ url: 'path:/it.at/not/../there', response: 200 });
expect(relativeRoute.matcher({ url: '/it.at/there' })).toBe(true);
});
it('dot segmentless path matches dot segmented path', () => {
const relativeRoute = new Route({ url: 'path:/it.at/there', response: 200 });
expect(relativeRoute.matcher({ url: '/it.at/not/../there' })).toBe(true);
});
})

})
});

0 comments on commit d01d7a9

Please sign in to comment.