Skip to content

Commit

Permalink
fix: return a json response when a incomplete url is requested
Browse files Browse the repository at this point in the history
  • Loading branch information
philip-hunt committed Oct 5, 2018
1 parent 3e50f9a commit c2ef892
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 11 deletions.
11 changes: 7 additions & 4 deletions dadi/lib/storage/disk.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ DiskStorage.prototype.getDefaultFile = function () {
let fullUrl = this.getFullUrl()

fs.lstat(fullUrl, (err, stats) => {
if (err) return reject(err)
if (err) return resolve()

if (stats.isDirectory()) {
let defaultFiles = config.get('defaultFiles')

fs.readdir(fullUrl, (err, files) => {
if (err) return reject(err)
if (err) return resolve()

files = files.filter(file => defaultFiles.includes(path.basename(file)))

Expand All @@ -58,7 +58,9 @@ DiskStorage.prototype.get = function () {

// If we're looking at a directory (assumed because no extension),
// attempt to get a configured default file from the directory
if (path.parse(this.getFullUrl()).ext === '') {
let isDirectory = path.parse(this.getFullUrl()).ext === ''

if (isDirectory) {
wait = this.getDefaultFile()
}

Expand Down Expand Up @@ -97,7 +99,8 @@ DiskStorage.prototype.get = function () {
}

return new Missing().get({
domain: this.domain
domain: this.domain,
isDirectory: isDirectory
}).then(stream => {
this.notFound = true
this.lastModified = new Date()
Expand Down
4 changes: 2 additions & 2 deletions dadi/lib/storage/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@ HTTPStorage.prototype.get = function ({

if (statusCode === 404) {
new Missing().get({
domain: this.domain
domain: this.domain,
isDirectory: path.parse(this.getFullUrl()).ext === ''
}).then(stream => {
this.notFound = true
this.lastModified = new Date()

resolve(stream)
}).catch(() => {
reject(httpError)
Expand Down
7 changes: 4 additions & 3 deletions dadi/lib/storage/missing.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,23 @@ const config = require(path.join(__dirname, '/../../../config'))

const Missing = function () {}

Missing.prototype.get = function ({domain} = {}) {
Missing.prototype.get = function ({domain, isDirectory = false}) {
let imagePath = config.get('notFound.images.enabled', domain)
? config.get('notFound.images.path', domain)
: null

return new Promise((resolve, reject) => {
if (!imagePath) {
if (!imagePath || isDirectory) {
return reject({ statusCode: 404 })
}

let stream = fs.createReadStream(imagePath)
let errorNotFound = {
statusCode: 404,
message: `File not found: ${imagePath}`
}

let stream = fs.createReadStream(imagePath)

stream.on('open', () => {
// Check file size.
fs.stat(imagePath, (error, stats) => {
Expand Down
3 changes: 2 additions & 1 deletion dadi/lib/storage/s3.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ S3Storage.prototype.get = function () {
(error) => {
if (error.statusCode === 404) {
return new Missing().get({
domain: this.domain
domain: this.domain,
isDirectory: path.parse(this.getFullUrl()).ext === ''
}).then(stream => {
this.notFound = true
this.lastModified = new Date()
Expand Down
47 changes: 46 additions & 1 deletion test/acceptance/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,27 @@ describe('Controller', function () {
})
})

it('should return a json response when a directory is requested', function (done) {

config.set('notFound.images.enabled', true)
config.set('notFound.images.path', './test/images/missing.png')
config.set('notFound.statusCode', 410)

let client = request(cdnUrl)
.get('/path/to/missing/')
.expect(410)
.end((err, res) => {
res.body.message.includes('File not found:').should.eql(true)
res.statusCode.should.eql(404)

config.set('notFound.images.enabled', configBackup.notFound.images.enabled)
config.set('notFound.statusCode', configBackup.notFound.statusCode)

done()
})

})

describe('when multi-domain is enabled', () => {
let fallbackImages = {
localhost: 'test/images/original.jpg',
Expand Down Expand Up @@ -1430,7 +1451,7 @@ describe('Controller', function () {
res.headers['content-type'].should.eql('image/png')
res.statusCode.should.eql(410)

config.set('notFound.images.enabled', configBackup.notFound.statusCode)
config.set('notFound.images.enabled', configBackup.notFound.images.enabled)
config.set('notFound.statusCode', configBackup.notFound.statusCode)

done()
Expand Down Expand Up @@ -1646,6 +1667,30 @@ describe('Controller', function () {
})
})

it('should return a json response when a directory is requested', function (done) {
// return 404 from the S3 request
AWS.mock('S3', 'getObject', Promise.reject({ statusCode: 404 }))

config.set('images.s3.bucketName', 'test-bucket')
config.set('images.s3.accessKey', 'xxx')
config.set('images.s3.secretKey', 'xyz')
config.set('notFound.statusCode', 404)
config.set('notFound.images.enabled', true)
config.set('notFound.images.path', './test/images/')

let client = request(cdnUrl)
.get('/images/mock/')
.expect(404)
.end((err, res) => {
AWS.restore()

res.body.message.includes('File not found:').should.eql(true)
res.statusCode.should.eql(404)

done()
})
})

it('should return configured statusCode if image is not found', function (done) {
// return 404 from the S3 request
AWS.mock('S3', 'getObject', Promise.reject({ statusCode: 404 }))
Expand Down

0 comments on commit c2ef892

Please sign in to comment.