Skip to content

Commit

Permalink
🐛 Fixed fallback to image-size when url contains uppercase ext or que…
Browse files Browse the repository at this point in the history
…ry param

no issue

- ensure we're downcasing the detected extension so that we can get a match against our "unprobable" formats
- adjust the extension matching regex to account for potential query parameters on the url
  • Loading branch information
kevinansfield committed Jun 11, 2019
1 parent cfd64d4 commit e998f2e
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
11 changes: 8 additions & 3 deletions lib/amperize.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ var EventEmitter = require('events').EventEmitter,
}
};

// these are formats supported by image-size but not probe-image-size
const FETCH_ONLY_FORMATS = [
'cur', 'icns', 'ico', 'dds'
];

/**
* Amperizer constructor. Borrows from Minimize.
*
Expand Down Expand Up @@ -176,10 +181,10 @@ Amperize.prototype.traverse = async function traverse(data, html, done) {
return Promise.resolve(imageSizeCache[url]);
}

var [, extension] = url.match(/(?:\.)([a-zA-Z]{3,4})$/) || [];

// fetch full image for formats we can't probe
if (['cur', 'icns', 'ico', 'dds'].includes(extension)) {
const extensionMatch = url.match(/(?:\.)([a-zA-Z]{3,4})(\?|$)/) || [];
const extension = (extensionMatch[1] || '').toLowerCase();
if (FETCH_ONLY_FORMATS.includes(extension)) {
return _fetchImageSize(url);
}

Expand Down
34 changes: 34 additions & 0 deletions test/amperize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,40 @@ describe('Amperize', function () {
});
});

it('falls back to image-size for unprobable images (uppercase extension)', function (done) {
imageSizeMock = nock('https://somewebsite.com')
.get('/favicon.ICO')
.replyWithFile(200, path.join(__dirname, 'fixtures/multi-size.ico'));

amperize.parse('<img src="https://somewebsite.com/favicon.ICO">', function (error, result) {
expect(result).to.exist;
expect(result).to.contain('<amp-img');
expect(result).to.contain('src="https://somewebsite.com/favicon.ICO"');
expect(result).to.contain('layout="fixed"');
expect(result).to.contain('width="256"');
expect(result).to.contain('height="256"');
expect(result).to.contain('</amp-img>');
done();
});
});

it('falls back to image-size for unprobable images (query param)', function (done) {
imageSizeMock = nock('https://somewebsite.com')
.get('/favicon.ICO?v=1')
.replyWithFile(200, path.join(__dirname, 'fixtures/multi-size.ico'));

amperize.parse('<img src="https://somewebsite.com/favicon.ICO?v=1">', function (error, result) {
expect(result).to.exist;
expect(result).to.contain('<amp-img');
expect(result).to.contain('src="https://somewebsite.com/favicon.ICO?v=1"');
expect(result).to.contain('layout="fixed"');
expect(result).to.contain('width="256"');
expect(result).to.contain('height="256"');
expect(result).to.contain('</amp-img>');
done();
});
});

it('returns largest image value for .ico files', function (done) {
imageSizeMock = nock('https://somewebsite.com')
.get('/favicon.ico')
Expand Down

0 comments on commit e998f2e

Please sign in to comment.