Fix/issue#1692

[zg009]

See here: #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

[TallTed]

human-facing language tweaks for clarity

[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

node-solid-server/lib/ldp.js

Line 331 in f5652f3

asynccheckItemName(url){

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

node-solid-server/lib/ldp.js

Line 150 in f5652f3

if(this.isAuxResource(slug,extension))throwerror(403,'POST is not allowed for auxiliary resources')

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 /
  

  node-solid-server/test/integration/http-test.js

  Line 838 in f5652f3

  it('should create container with new slug as a resource',function(done){

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]

some console in lib/ldp.js to remove

[bourgeoa]

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

[bourgeoa]

LGTM

[TallTed]

human facing, relatively small

[zg009]

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

[TallTed]

OK by me. Note that I code primarily in English, and cannot provide much if any useful input on the JavaScript or other code here.

[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.