Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid URL Fix: Decoding manually for some reserved characters #1682

Merged
merged 14 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/core/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ export const configSchema = {
type: 'boolean',
default: true
},
modifySnapshotUrl: {
type: 'boolean',
default: true
},
disableShadowDOM: {
type: 'boolean',
default: false
Expand Down Expand Up @@ -279,6 +283,7 @@ export const snapshotSchema = {
percyCSS: { $ref: '/config/snapshot#/properties/percyCSS' },
enableJavaScript: { $ref: '/config/snapshot#/properties/enableJavaScript' },
cliEnableJavaScript: { $ref: '/config/snapshot#/properties/cliEnableJavaScript' },
modifySnapshotUrl: { $ref: '/config/snapshot#/properties/modifySnapshotUrl' },
disableShadowDOM: { $ref: '/config/snapshot#/properties/disableShadowDOM' },
domTransformation: { $ref: '/config/snapshot#/properties/domTransformation' },
enableLayout: { $ref: '/config/snapshot#/properties/enableLayout' },
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/discovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ function debugSnapshotOptions(snapshot) {
debugProp(snapshot, 'enableJavaScript');
debugProp(snapshot, 'cliEnableJavaScript');
debugProp(snapshot, 'disableShadowDOM');
debugProp(snapshot, 'modifySnapshotUrl');
debugProp(snapshot, 'enableLayout');
debugProp(snapshot, 'domTransformation');
debugProp(snapshot, 'reshuffleInvalidTags');
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ function mapSnapshotOptions(snapshots, context) {
// transform snapshot URL shorthand into an object
if (typeof snapshot === 'string') snapshot = { url: snapshot };

validateAndFixSnapshotUrl(snapshot);
if (snapshot.modifySnapshotUrl !== false && process.env.PERCY_MODIFY_SNAPSHOT_URL !== 'false') validateAndFixSnapshotUrl(snapshot);

let url = validURL(snapshot.url, context?.baseUrl);
// normalize the snapshot url and use it for the default name
Expand Down
26 changes: 25 additions & 1 deletion packages/core/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,29 @@ export function base64encode(content) {
.toString('base64');
}

const RESERVED_CHARACTERS = {
'%3A': ':',
'%23': '#',
'%24': '$',
'%26': '&',
'%2B': '+',
'%2C': ',',
'%2F': '/',
'%3B': ';',
'%3D': '=',
'%3F': '?',
'%40': '@'
};

function _replaceReservedCharacters(url) {
let result = url;
for (let [key, value] of Object.entries(RESERVED_CHARACTERS)) {
let regex = new RegExp(key, 'g');
result = result.replace(regex, value);
}
return result;
}

// This function replaces invalid character that are not the
// part of valid URI syntax with there correct encoded value.
// Also, if a character is a part of valid URI syntax, those characters
Expand All @@ -388,7 +411,8 @@ export function decodeAndEncodeURLWithLogging(url, logger, options = {}) {
// correctly.
const { meta, shouldLogWarning, warningMessage } = options;
try {
let decodedURL = decodeURI(url); // This can throw error, so handle it will trycatch
let decodedURL = _replaceReservedCharacters(url);
decodedURL = decodeURI(decodedURL);
let encodedURL = encodeURI(decodedURL);
return encodedURL;
} catch (error) {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/test/percy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ describe('Percy', () => {
percyCSS: '',
enableJavaScript: false,
disableShadowDOM: false,
cliEnableJavaScript: true
cliEnableJavaScript: true,
modifySnapshotUrl: true
});
});

Expand Down
57 changes: 57 additions & 0 deletions packages/core/test/snapshot.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,63 @@ describe('Snapshot', () => {
sharedExpectModifiedSnapshotURL('http://localhost:8000/advanced-search/cf1a5848-f658-4939-be11-dctwo-%7B%20abc%20%5Bdbd%5D%20%7D/2253');
});

it('do not double encode reserved URI character(%2F) in snapshot url', async () => {
let partiallyEncodedURL = 'http://localhost:8000/https%2F/abc[123].html';
await percy.snapshot([
{ url: partiallyEncodedURL }
]);

sharedExpectModifiedSnapshotURL('http://localhost:8000/https//abc%5B123%5D.html');
});

describe('does not modifies snapshot url', () => {
let partiallyEncodedURL = 'http://localhost:8000/https%2F/abc[123].html';
const sharedExpect = () => {
expect(logger.stderr).toEqual(jasmine.arrayContaining([
'[percy:core:snapshot] Received snapshot: /https%2F/abc[123].html',
'[percy:core:snapshot] - url: http://localhost:8000/https%2F/abc[123].html'
]));

expect(logger.stderr).not.toEqual(jasmine.arrayContaining([
'[percy:core:snapshot] Snapshot URL modified to: http://localhost:8000/https//abc%5B123%5D.html'
]));
};

it('when modifySnapshotUrl set to false', async () => {
await percy.snapshot([
{ url: partiallyEncodedURL, modifySnapshotUrl: false }
]);

sharedExpect();
});

describe('with PERCY_MODIFY_SNAPSHOT_URL=false', () => {
beforeEach(() => {
process.env.PERCY_MODIFY_SNAPSHOT_URL = 'false';
});

afterEach(() => {
process.env.PERCY_MODIFY_SNAPSHOT_URL = 'true';
});

it('uses original snapshot url', async () => {
await percy.snapshot([
{ url: partiallyEncodedURL }
]);

sharedExpect();
});

it('uses original snapshot url if modifySnapshotUrl = false ', async () => {
await percy.snapshot([
{ url: partiallyEncodedURL, modifySnapshotUrl: false }
]);

sharedExpect();
});
});
});

it('warns for invalid characters that can not get encoded properly', async () => {
let givenURL = 'http://localhost:8000/advanced-search/cf1a5848%-f658-4939-be11-dct';
await percy.snapshot([
Expand Down
1 change: 1 addition & 0 deletions packages/core/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ interface CommonSnapshotOptions {
percyCSS?: string;
enableJavaScript?: boolean;
disableShadowDOM?: boolean;
modifySnapshotUrl?: boolean;
enableLayout?: boolean;
domTransformation?: string;
reshuffleInvalidTags?: boolean;
Expand Down
Loading