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

update check itemName #1748

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
6 changes: 5 additions & 1 deletion lib/handlers/patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ async function patchHandler (req, res, next) {
({ path, contentType } = await ldp.resourceMapper.mapUrlToFile(
{ url: req, createIfNotExists: true, contentType: contentTypeForNew(req) }))
// check if a folder with same name exists
await ldp.checkItemName(req)
try {
await ldp.checkItemName(req)
} catch (err) {
return next(err)
}
resourceExists = false
}
const { url } = await ldp.resourceMapper.mapFileToUrl({ path, hostname: req.hostname })
Expand Down
9 changes: 8 additions & 1 deletion lib/handlers/put.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,15 @@ const { stringToStream } = require('../utils')
async function handler (req, res, next) {
debug(req.originalUrl)
res.header('MS-Author-Via', 'SPARQL')

const contentType = req.get('content-type')

// check if a folder or resource with same name exists
bourgeoa marked this conversation as resolved.
Show resolved Hide resolved
try {
const ldp = req.app.locals.ldp
await ldp.checkItemName(req)
} catch (e) {
return next(e)
}
// check for valid rdf content for auxiliary resource and /profile/card
// in future we may check that /profile/card is a minimal valid WebID card
if (isAuxiliary(req) || req.originalUrl === '/profile/card') {
Expand Down
68 changes: 25 additions & 43 deletions lib/ldp.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,37 +157,13 @@ class LDP {
if (container) {
extension = ''
}
// Check for file or folder with same name
let testName, fileName
try {
// check for defaulted slug in NSS POST (slug with extension)
fileName = slug.endsWith(extension) || slug.endsWith(this.suffixAcl) || slug.endsWith(this.suffixMeta) ? slug : slug + extension
fileName = container ? fileName : fileName + '/'
const { url: testUrl } = await this.resourceMapper.mapFileToUrl({ hostname, path: containerPath + fileName })
const { path: testPath } = await this.resourceMapper.mapUrlToFile({ url: testUrl })
testName = container ? fs.lstatSync(testPath).isFile() : fs.lstatSync(testPath).isDirectory()
} catch (err) { testName = false }

if (testName) {
throw error(404, 'Container and resource cannot have the same name in URI')
}

// TODO: possibly package this in ldp.post
let resourceUrl = await ldp.getAvailableUrl(hostname, containerPath, { slug, extension })
// allways return a valid URL.
bourgeoa marked this conversation as resolved.
Show resolved Hide resolved
const resourceUrl = await ldp.getAvailableUrl(hostname, containerPath, { slug, extension, container })
debug.handlers('POST -- Will create at: ' + resourceUrl)
let originalUrl = resourceUrl

if (container) {
// Create directory by an LDP PUT to the container's .meta resource
resourceUrl = `${resourceUrl}${resourceUrl.endsWith('/') ? '' : '/'}` // ${ldp.suffixMeta}`
if (originalUrl && !originalUrl.endsWith('/')) {
originalUrl += '/'
}
}
// const { url: putUrl } = await this.resourceMapper.mapFileToUrl({ path: resourceUrl, hostname })

await ldp.put(resourceUrl, stream, contentType)
return URL.parse(originalUrl).path
return URL.parse(resourceUrl).path
}

isAuxResource (slug, extension) {
Expand Down Expand Up @@ -249,10 +225,6 @@ class LDP {
throw error(400, 'Resource with a $.ext is not allowed by the server')
}

// check if a folder or file with same name exists
// const urlItem = url.url || url
await this.checkItemName(url)

// First check if we are above quota
let isOverQuota
// Someone had a reason to make url actually a req sometimes but not
Expand Down Expand Up @@ -355,8 +327,9 @@ class LDP {
} catch (err) { }
}

// check if a document (or container) has the same name than a document (or container)
bourgeoa marked this conversation as resolved.
Show resolved Hide resolved
async checkItemName (url) {
let testName
let testName, testPath
const { hostname, pathname } = this.resourceMapper._parseUrl(url) // (url.url || url)
let itemUrl = this.resourceMapper.resolveUrl(hostname, pathname)
const container = itemUrl.endsWith('/')
Expand All @@ -373,11 +346,11 @@ class LDP {
const { pathname } = this.resourceMapper._parseUrl(itemUrl) // (url.url || url)
// check not at root
if (pathname !== '/') {
await this.checkItemName(itemUrl)
return await this.checkItemName(itemUrl)
}
}
if (testName) {
throw error(404, 'Container and resource cannot have the same name in URI')
throw error(404, `${testPath}: Container and resource cannot have the same name in URI`)
}
}

Expand Down Expand Up @@ -606,17 +579,26 @@ class LDP {
}
}

async getAvailableUrl (hostname, containerURI, { slug = uuid.v1(), extension }) {
async getAvailableUrl (hostname, containerURI, { slug = uuid.v1(), extension, container }) {
let requestUrl = this.resourceMapper.resolveUrl(hostname, containerURI)
requestUrl = requestUrl.replace(/\/*$/, '/')
requestUrl = requestUrl.replace(/\/*$/, '/') // ??? what for

const { path: containerFilePath } = await this.resourceMapper.mapUrlToFile({ url: requestUrl })
let fileName = slug.endsWith(extension) || slug.endsWith(this.suffixAcl) || slug.endsWith(this.suffixMeta) ? slug : slug + extension
if (await promisify(fs.exists)(utilPath.join(containerFilePath, fileName))) {
fileName = `${uuid.v1()}-${fileName}`
}

return requestUrl + fileName
let itemName = slug.endsWith(extension) || slug.endsWith(this.suffixAcl) || slug.endsWith(this.suffixMeta) ? slug : slug + extension
try {
// check resource exists
bourgeoa marked this conversation as resolved.
Show resolved Hide resolved
const context = container ? '/' : ''
await this.resourceMapper.mapUrlToFile({ url: (requestUrl + itemName + context) })
itemName = `${uuid.v1()}-${itemName}`
} catch (e) {
try {
// check resource with same name exists
bourgeoa marked this conversation as resolved.
Show resolved Hide resolved
const context = !container ? '/' : ''
await this.resourceMapper.mapUrlToFile({ url: (requestUrl + itemName + context) })
itemName = `${uuid.v1()}-${itemName}`
} catch (e) {}
}
if (container) itemName += '/'
return requestUrl + itemName
}

getTrustedOrigins (req) {
Expand Down
Loading
Loading