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

User/Track update ipfs bug fix #1029

Merged
merged 11 commits into from
Nov 5, 2020
4 changes: 2 additions & 2 deletions creator-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
"sequelize": "^4.41.2",
"shortid": "^2.2.14",
"umzug": "^2.2.0",
"uuid": "3.3.2"
"uuid": "3.3.2",
"fs-extra": "^9.0.1"
},
"devDependencies": {
"mocha": "^5.2.0",
Expand All @@ -61,7 +62,6 @@
"sequelize-cli": "^5.3.0",
"sinon": "^7.0.0",
"standard": "^12.0.1",
"fs-extra": "^9.0.1",
"supertest": "^3.3.0",
"proxyquire": "^2.1.3"
},
Expand Down
8 changes: 5 additions & 3 deletions creator-node/src/routes/audiusUsers.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const fs = require('fs')
const models = require('../models')
const { saveFileFromBufferToIPFSAndDisk } = require('../fileManager')
const { handleResponse, successResponse, errorResponseBadRequest, errorResponseServerError } = require('../apiHelpers')
const { getFileUUIDForImageCID } = require('../utils')
const { validateStateForImageDirCIDAndReturnFileUUID } = require('../utils')
const { authMiddleware, syncLockMiddleware, ensurePrimaryMiddleware, triggerSecondarySyncs } = require('../middlewares')
const DBManager = require('../dbManager')

Expand Down Expand Up @@ -85,8 +85,10 @@ module.exports = function (app) {
// Get coverArtFileUUID and profilePicFileUUID for multihashes in metadata object, if present.
let coverArtFileUUID, profilePicFileUUID
try {
coverArtFileUUID = await getFileUUIDForImageCID(req, metadataJSON.cover_photo_sizes)
profilePicFileUUID = await getFileUUIDForImageCID(req, metadataJSON.profile_picture_sizes)
[coverArtFileUUID, profilePicFileUUID] = await Promise.all(
validateStateForImageDirCIDAndReturnFileUUID(req, metadataJSON.cover_photo_sizes),
validateStateForImageDirCIDAndReturnFileUUID(req, metadataJSON.profile_picture_sizes)
)
} catch (e) {
return errorResponseBadRequest(e.message)
}
Expand Down
50 changes: 49 additions & 1 deletion creator-node/src/routes/files.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const Redis = require('ioredis')
const fs = require('fs')
const { promisify } = require('util')
const fs = require('fs-extra')
const fsStat = promisify(fs.stat)
SidSethi marked this conversation as resolved.
Show resolved Hide resolved
const path = require('path')
var contentDisposition = require('content-disposition')

Expand Down Expand Up @@ -353,6 +355,52 @@ module.exports = function (app) {
return errorResponseServerError(e)
}

/**
* Data validation for resize image response
* - Ensure dir exists on disk for dirCID
* - Check dir contents with ipfs.ls
* - Confirm all files are stored on disk in expected location and have expected size
*/
try {
const ipfs = req.app.get('ipfsLatestAPI')

const resizeRespFileMap = resizeResp.files.reduce((map, obj) => (map[obj.multihash] = obj, map), {})

const dirCID = resizeResp.dir.dirCID

// Ensure dir exists on disk
if (!(await fs.pathExists(resizeResp.dir.dirDestPath))) {
throw new Error(`No dir found on disk for dir CID ${dirCID} at expected path ${resizeResp.dir.dirDestPath}`)
}

// Ensure each imageCID exists on disk with expected size and matches data from resizeResp object
let fileCount = 0
for await (const file of ipfs.ls(dirCID, { timeout: 500 })) {
if (file.type !== 'file' || file.depth !== 1) {
throw new Error(`Found unexpected non-file contents in dir ${dirCID}`)
}
const cid = file.cid.toString()
const resizeRespFileObj = resizeRespFileMap[cid]

if (!resizeRespFileObj) {
throw new Error(`resizeResp object for dir ${dirCID} missing data for file ${cid}`)
}

// fsStat(filePath) will throw error if no file exists at filePath
const fileStat = await fsStat(resizeRespFileObj.storagePath)
if (fileStat.size !== file.size) {
throw new Error(`File on disk has unexpected size - expected: ${file.size} - actual ${fileStat.size}`)
}

fileCount++
}
if (fileCount != resizeResp.files.length) {
throw new Error(`IPFS Dir contents don't match resizeImage response for dir CID ${dirCID} - expected length: ${resizeResp.files.length} - actual length: ${fileCount}`)
}
} catch (e) {
throw new Error(`IPFS Validation failed - ${e.message}`)
}

// Record image file entries in DB
const transaction = await models.sequelize.transaction()
try {
Expand Down
2 changes: 1 addition & 1 deletion creator-node/src/routes/nodeSync.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ async function _nodesync (req, walletPublicKeys, creatorNodeEndpoint, dbOnlySync
// Spread + set uniq's the array
userReplicaSet = [...new Set(userReplicaSet)]
} catch (e) {
req.logger.error(redisKey, `Couldn't get user's replica sets, can't use cnode gateways in saveFileForMultihash`)
req.logger.error(redisKey, `Couldn't get user's replica sets, can't use cnode gateways in saveFileForMultihash - ${e.message}`)
SidSethi marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
4 changes: 2 additions & 2 deletions creator-node/src/routes/tracks.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const {
errorResponseServerError,
errorResponseForbidden
} = require('../apiHelpers')
const { getFileUUIDForImageCID } = require('../utils')
const { validateStateForImageDirCIDAndReturnFileUUID } = require('../utils')
const { authMiddleware, ensurePrimaryMiddleware, syncLockMiddleware, triggerSecondarySyncs } = require('../middlewares')
const TranscodingQueue = require('../TranscodingQueue')
const { getCID } = require('./files')
Expand Down Expand Up @@ -310,7 +310,7 @@ module.exports = function (app) {
// Get coverArtFileUUID for multihash in metadata object, else error
let coverArtFileUUID
try {
coverArtFileUUID = await getFileUUIDForImageCID(req, metadataJSON.cover_art_sizes)
coverArtFileUUID = await validateStateForImageDirCIDAndReturnFileUUID(req, metadataJSON.cover_art_sizes)
} catch (e) {
return errorResponseServerError(e.message)
}
Expand Down
82 changes: 40 additions & 42 deletions creator-node/src/utils.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
const { recoverPersonalSignature } = require('eth-sig-util')
const { promisify } = require('util')
const fs = require('fs')
const fs = require('fs-extra')
const path = require('path')
const mkdir = promisify(fs.mkdir)
const { BufferListStream } = require('bl')
const axios = require('axios')

Expand All @@ -26,47 +24,47 @@ class Utils {
}
}

async function getFileUUIDForImageCID (req, imageCID) {
const ipfs = req.app.get('ipfsAPI')
if (imageCID) { // assumes imageCIDs are optional params
// Ensure CID points to a dir, not file
let cidIsFile = false
try {
await ipfs.cat(imageCID, { length: 1 })
cidIsFile = true
} catch (e) {
// Ensure file exists for dirCID
const dirFile = await models.File.findOne({
where: { multihash: imageCID, cnodeUserUUID: req.session.cnodeUserUUID, type: 'dir' }
})
if (!dirFile) {
throw new Error(`No file stored in DB for dir CID ${imageCID}`)
}
/**
* Ensure DB and disk records exist for dirCID and its contents
* Return fileUUID for dir DB record
* This function does not do further validation since image_upload provides remaining guarantees
*/
async function validateStateForImageDirCIDAndReturnFileUUID (req, imageDirCID) {
SidSethi marked this conversation as resolved.
Show resolved Hide resolved
req.logger.info(`Beginning validateStateForImageDirCIDAndReturnFileUUID for imageDirCID ${imageDirCID}`)

// Ensure file refs exist in DB for every file in dir
const dirContents = await ipfs.ls(imageCID)
req.logger.info(dirContents)
if (!imageDirCID) {
return null
SidSethi marked this conversation as resolved.
Show resolved Hide resolved
}

// Iterates through directory contents but returns upon first iteration
// TODO: refactor to remove for-loop
for (let fileObj of dirContents) {
if (!fileObj.hasOwnProperty('hash') || !fileObj.hash) {
throw new Error(`Malformatted dir contents for dirCID ${imageCID}. Cannot process.`)
}
// Ensure file exists for dirCID
const dirFile = await models.File.findOne({
where: { multihash: imageDirCID, cnodeUserUUID: req.session.cnodeUserUUID, type: 'dir' }
})
if (!dirFile) {
throw new Error(`No file stored in DB for imageDirCID ${imageDirCID}`)
}

const imageFile = await models.File.findOne({
where: { multihash: fileObj.hash, cnodeUserUUID: req.session.cnodeUserUUID, type: 'image' }
})
if (!imageFile) {
throw new Error(`No file ref stored in DB for CID ${fileObj.hash} in dirCID ${imageCID}`)
}
return dirFile.fileUUID
}
}
if (cidIsFile) {
throw new Error(`CID ${imageCID} must point to a valid directory on IPFS`)
// Ensure dir exists on disk
if (!(await fs.pathExists(dirFile.storagePath))) {
throw new Error(`No dir found on disk for imageDirCID ${imageDirCID} at expected path ${dirFile.storagePath}`)
}

const imageFiles = await models.File.findAll({
where: { dirMultihash: imageDirCID, cnodeUserUUID: req.session.cnodeUserUUID, type: 'image' }
})
if (!imageFiles) {
throw new Error(`No image file records found in DB for imageDirCID ${imageDirCID}`)
}

// Ensure every file exists on disk
await Promise.all(imageFiles.map(async function (imageFile) {
if(!(await fs.pathExists(imageFile.storagePath))) {
throw new Error(`No file found on disk for imageDirCID ${imageDirCID} image file at path ${imageFile.path}`)
}
} else return null
}))

req.logger.info(`Completed validateStateForImageDirCIDAndReturnFileUUID for imageDirCID ${imageDirCID}`)
return dirFile.fileUUID
}

async function getIPFSPeerId (ipfs) {
Expand Down Expand Up @@ -417,7 +415,7 @@ async function rehydrateIpfsDirFromFsIfNecessary (dirHash, logContext) {

async function createDirForFile (fileStoragePath) {
const dir = path.dirname(fileStoragePath)
await mkdir(dir, { recursive: true })
await fs.ensureDir(dir)
SidSethi marked this conversation as resolved.
Show resolved Hide resolved
}

async function writeStreamToFileSystem (stream, expectedStoragePath, createDir = false) {
Expand All @@ -437,7 +435,7 @@ async function writeStreamToFileSystem (stream, expectedStoragePath, createDir =
}

module.exports = Utils
module.exports.getFileUUIDForImageCID = getFileUUIDForImageCID
module.exports.validateStateForImageDirCIDAndReturnFileUUID = validateStateForImageDirCIDAndReturnFileUUID
module.exports.getIPFSPeerId = getIPFSPeerId
module.exports.rehydrateIpfsFromFsIfNecessary = rehydrateIpfsFromFsIfNecessary
module.exports.rehydrateIpfsDirFromFsIfNecessary = rehydrateIpfsDirFromFsIfNecessary
Expand Down