Fix/issue#1692

[zg009]

See here: https://github.com/nodeSolidServer/node-solid-server/issues/1692

Added a check in put.js handler to see if a client is attempting to create an invalid resource uri

Alain, before this gets merged, let me know if there is a better way to get the RESERVED_SUFFIXES, and if the function should be moved to ldp.js instead

[bourgeoa]

@zg009 Thanks for looking at this issue.

• Both PUT and PATCH are allowed to create intermediate Container
• PUT can create Container, may fail
• POST can create Container with slug, never fails and return location

The container name

1. cannot be the same as an existing resource name
2. cannot end with a reserved auxiliary extension, to not forbid creation of an auxiliary resource

https://github.com/nodeSolidServer/node-solid-server/blob/f5652f3dfa3ae630408125af99bcf90b7fe21c0d/lib/ldp.js#L331 is where point 1. is checked.

Point 2. may be added in this above function

[bourgeoa]

@zg009 Not sure of POST creating a container with slug. I don't think sure such a test exist https://github.com/nodeSolidServer/node-solid-server/blob/f5652f3dfa3ae630408125af99bcf90b7fe21c0d/lib/ldp.js#L150 Could you add a test for POST trying to create a Container with an unallowed slug

[zg009]

There are tests for invalid slug on POST requests here: https://github.com/nodeSolidServer/node-solid-server/blob/f5652f3dfa3ae630408125af99bcf90b7fe21c0d/test/integration/http-test.js#L863C4-L876C7

I added the check I believe you requested

[bourgeoa]

Sorry I am on a tablet. Not easy to code

• You are testing a POST creating a resource. I don't see why it should fail. POST never fail. The name of the resource created must be checked on the location header. Solid specification introduced an exception 409 for auxiliary resources because you cannot be unsure on the resource name in auxiliary resources

• You are not testing a POST creating a container with a slug ending with .acl Below an exemple. Notice the link that say I want a container. This is explicitly needed because a slug cannot end with / https://github.com/nodeSolidServer/node-solid-server/blob/f5652f3dfa3ae630408125af99bcf90b7fe21c0d/test/integration/http-test.js#L838

The test should create a container. The container name can be found in location header see next test.

Your code in ldp.js should do something like

If (container) {
  extension = ''
 If slug ends with unallowed extension remove unallowed extension from slug
}
elseif (slug) test slug to not allow creation of auxiliary --> 409

[zg009]

Alain, I do not see how you can do what you are stating is possible.

If slug has extension and extension is .acl or .meta, 403 is thrown server.post('/post-resources/').set('slug', 'post-resource.acl') // throws 403 server.post('/post-resources/').set('slug', 'post-resource.meta') // throws 403

If slug has no extension, it becomes container. server.post('/post-resources/').set('slug', 'inner-container') // creates container /post-resources/inner-container/

I do not know how the situation you are describing can exist.

[bourgeoa]

@zg009 Could you review at my changes Basically

• the issue is only for container creation
• the check is only at the extension part (endsWith)

Can you

• add a test for .meta
• TODO find a better test name on line 894

[zg009]

@bourgeoa I renamed it and added some clarity on the new .meta and .acl tests

[bourgeoa]

Thanks LGTM just remove the 2 console.log() in the recursive

[zg009]

@TallTed If it looks good to you, can you go ahead and approve the changes and I'll merge

[zg009]

@TallTed That is OK. It is nice to have more input regarding clarity, when a test or code change has to be revisited later.