Skip to content

fix(cdn): MERC-9364 Use CDN Original images for webdav - cache control #298

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(cdn): MERC-9364 Use CDN Original images for webdav - cache control #298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 8 additions & 1 deletion helpers/lib/cdnify.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,14 @@ module.exports = globals => {
}

if (protocol === 'webdav:') {
return [cdnUrl, 'content', path].join('/');
const supportedImageExtensions = ['jpg', 'jpeg', 'gif', 'png'];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include . ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think no, Jay. On L61 I exclude any searchParams from path and what is left I check if it ends with supported extension. So for example User provides something like: webdav:/collection/gifts/image.pdf?cache-buster=2025-06-04T00:50:40.
We then check first if it webdav, then go and look for any searchParams and drop them so receive path: /collection/gifts/image.pdf and then check if it ends with supported extensions.

const unsupportedPathSyntaxes = ['../', './'];
const searchParamIgnoredPath = path.split('?')[0];
Copy link

@bc-alexsaiannyi bc-alexsaiannyi Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should cover cases when pathname contains searchParams like
'webdav:/img/brands-logos/main.png?cache_bust=1749005114992'
and exclude them from filtering on the next step

const isImage = supportedImageExtensions.some(ext => searchParamIgnoredPath.toLowerCase().endsWith(ext));
Copy link

@bc-alexsaiannyi bc-alexsaiannyi Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previously used method includes() can lead to incorrect results. For example, if provided path looks like 'webdav:banners/menu-giftcard.svg' or 'webdav:/collection/gifts/image.pdf?cache-buster=2025-06-04T00:50:40' then we will get situation where path.toLowerCase().includes(ext)) return true and helper modified path for unsupported content.

Now the logic has been updated, so firstly we exclude searchParams from path and then check if it ends with supported extension.

const isTraversedPathUsed = unsupportedPathSyntaxes.some(syntax => searchParamIgnoredPath.startsWith(syntax));
// to avoid breaking changes, we keep paths for webdav content unchanged if the path is traversed
const prefix = isImage && !isTraversedPathUsed ? 'images/stencil/original/content' : 'content'
return [cdnUrl, prefix, path].join('/');
}

if (cdnSettings) {
Expand Down
53 changes: 47 additions & 6 deletions spec/helpers/cdn.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,19 +194,60 @@ describe('cdn helper', function () {
], done);
});

it('should return a webDav asset if webdav protocol specified', function (done) {
it('should return an original cdn img asset if webdav protocol specified but file type indicates it is an image', function (done) {
runTestCases([
{
input: '{{cdn "webdav:img/image.jpg"}}',
output: 'https://cdn.bcapp/3dsf74g/content/img/image.jpg',
output: 'https://cdn.bcapp/3dsf74g/images/stencil/original/content/img/image.jpg',
},
{
input: '{{cdn "webdav:/img/image.jpg"}}',
output: 'https://cdn.bcapp/3dsf74g/content/img/image.jpg',
input: '{{cdn "webdav:/img/image.jpeg"}}',
output: 'https://cdn.bcapp/3dsf74g/images/stencil/original/content/img/image.jpeg',
},
{
input: '{{cdn "webdav://img/image.jpg"}}',
output: 'https://cdn.bcapp/3dsf74g/content/img/image.jpg',
input: '{{cdn "webdav://img/image.gif"}}',
output: 'https://cdn.bcapp/3dsf74g/images/stencil/original/content/img/image.gif',
},
{
input: '{{cdn "webdav://img/image.png"}}',
output: 'https://cdn.bcapp/3dsf74g/images/stencil/original/content/img/image.png',
},
{
input: '{{cdn "webdav:img/image.jpg?040620250934"}}',
output: 'https://cdn.bcapp/3dsf74g/images/stencil/original/content/img/image.jpg?040620250934',
},
{
input: '{{cdn "webdav:img/image.png?cache_bust=1749005114992"}}',
output: 'https://cdn.bcapp/3dsf74g/images/stencil/original/content/img/image.png?cache_bust=1749005114992',
},
{
input: '{{cdn "webdav:img/image.png?040620250934&cache_bust=1749005114992"}}',
output: 'https://cdn.bcapp/3dsf74g/images/stencil/original/content/img/image.png?040620250934&cache_bust=1749005114992',
},
{
input: '{{cdn "webdav:img/image.png?foo=bar&cache_bust=1749005114992"}}',
output: 'https://cdn.bcapp/3dsf74g/images/stencil/original/content/img/image.png?foo=bar&cache_bust=1749005114992',
},
], done);
});

it('should return a webDav asset if webdav protocol specified but is not a supported image type', function (done) {
runTestCases([
{
input: '{{cdn "webdav:img/image.pdf"}}',
output: 'https://cdn.bcapp/3dsf74g/content/img/image.pdf',
},
{
input: '{{cdn "webdav:/img/image.pdf"}}',
output: 'https://cdn.bcapp/3dsf74g/content/img/image.pdf',
},
{
input: '{{cdn "webdav://img/image.pdf"}}',
output: 'https://cdn.bcapp/3dsf74g/content/img/image.pdf',
},
{
input: '{{cdn "webdav://gifts/image.pdf"}}',
output: 'https://cdn.bcapp/3dsf74g/content/gifts/image.pdf',
},
], done);
});
Expand Down