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

POST does not work the way it did in 4.x #1246

Open
jeff-zucker opened this issue Jul 1, 2019 · 10 comments

Comments

4 participants
@jeff-zucker
Copy link

commented Jul 1, 2019

From my testing (test attached below) I notice these differences:

POST "foo", no content-type                                                     
  4.x : "foo" text/turtle                                                       
  5.x : 500                                                                     
POST "foo.ttl", no content-type                                                 
  4.x : "foo.ttl" text/turtle                                                   
  5.x : 500                                                                     
POST "foo", content-type=text/turtle                                            
  4.x : "foo"     text/turtle                                                   
  5.x : "foo.ttl" text/turtle                                                   
POST "foo.ttl", content-type=text/turtle                                        
  4.x : "foo.ttl.ttl" text/turtle                                               
  5.x : "foo.ttl.ttl" text/turtle                                               

Here's the test. I ran it against solid.community for 5.x and against solid.openlinksw.com:8444 for 4.x

const auth       = require('solid-auth-cli')
const text       = "<> a <#test>."

const idp  = "https://jeffz.solid.community"
const base = idp + "/public/nss5-test/"
const file = "test-file"

async function main(){
  let session = await auth.login()
  console.log("Logged in as <"+session.webId+">")
  console.log("Using folder <"+base+">\n")
  await test({
     label       : "POST, no content-type",
     extension   : "",
     contentType : ""
  })
  await test({
     label       : "POST content-type=text/turtle",
     extension   : "",
     contentType : "text/turtle"
  })
  await test({
     label       : "POST, no content-type",
     extension   : ".ttl",
     contentType : ""
  })

}
async function test(opts){
  let url = file + opts.extension
  console.log("    ",opts.label,"<"+url+">")
  let headers = {
    link : '<http://www.w3.org/ns/ldp#Resource>; rel="type"',
    slug : url,
  }
  if(opts.contentType) headers["Content-type"]=opts.contentType
  let res = await auth.fetch( base, {
    method:"POST",
    body:text,
    headers:headers
  })
  if(!res.ok){
    console.log("        ",res.status,res.statusText)
    return
  }
 let location = idp + res.headers.get("location")
 res = await auth.fetch( location )
 location = location.replace(base,'')
 if(res.ok) console.log(
      "        ",
    res.status,
    res.headers.get("content-type"),
    location
  )
  else console.log("        ",res.status,res.statusText)
}
main()
@Otto-AA

This comment has been minimized.

Copy link

commented Jul 1, 2019

Edit: I've misread the OP, following is slightly off topic...

I've already mentioned this behaviour in this issue, that the slug and the mapped content-type just get joined together. So having slug: file.xyz and Content-Type: text/turtle will result in file.xyz.ttl

Here is the part of the code which I think causes this (in particular line 440):

async getAvailableUrl (hostname, containerURI, { slug = uuid.v1(), extension }) {
let requestUrl = this.resourceMapper.resolveUrl(hostname, containerURI)
requestUrl = requestUrl.replace(/\/*$/, '/')
const { path: containerFilePath } = await this.resourceMapper.mapUrlToFile({ url: requestUrl })
let fileName = slug + extension
if (await promisify(fs.exists)(utilPath.join(containerFilePath, fileName))) {
fileName = `${uuid.v1()}-${fileName}`
}
return requestUrl + fileName
}

@jaxoncreed jaxoncreed added this to To do in ASAP on Server via automation Jul 1, 2019

@jeff-zucker

This comment has been minimized.

Copy link
Author

commented Jul 1, 2019

@Otto-AA's point is a good one and I support following up on that. Ironically, that's the only one of the four I posted that matches 4.x behavior. Tim has said #1165 (comment) that PUT without content-type is not valid and I'm wondering if this also applies to POST, in which case the 500 in the first two cases is correct though not-backward compatible. The third case above is concerning to me because it means it is impossible to create a Turtle resource that has no extension - if it isn't valid to create without sending a content-type and sending a content-type always tacks on an extension that means all resources have extensions. Perhaps this is by design, but it bears some thought.

@Otto-AA

This comment has been minimized.

Copy link

commented Jul 1, 2019

Regarding the 500 response if no Content-Type is provided, I want to add that I would expect a 400 response in such a case. Apart from that I completely agree with @jeff-zucker that clarification on this issue would be helpful.

@jeff-zucker

This comment has been minimized.

Copy link
Author

commented Jul 1, 2019

Yes, 400 is more accurate than 500.

@jeff-zucker

This comment has been minimized.

Copy link
Author

commented Jul 2, 2019

Another serious problem reported by a user : how does one create foo.acl? It fails with no content-type and creates foo.acl.ttl with content-type "text/turtle".

@jeff-zucker

This comment has been minimized.

Copy link
Author

commented Jul 2, 2019

To summarize what I'd like to see:

  • PUT or POST with no content-type specified
     * either 400 or 415 
     * or if you feel forgiving, default to text/turtle :-)
  • PUT or POST with content-type specified
    * create with that content-type
    * server does not ever append an extension

All assuming the user has write permissions.

@kjetilk kjetilk added the test-case label Jul 4, 2019

@RubenVerborgh

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

Another serious problem reported by a user : how does one create foo.acl? It fails with no content-type and creates foo.acl.ttl with content-type "text/turtle".

PUT or POST?

It should be done with PUT, but I can very well imagine that failing currently if the .acl extension is not treated as a special case.

@jeff-zucker

This comment has been minimized.

Copy link
Author

commented Jul 8, 2019

@RubenVerborgh Yes, PUT can create resources with no extensions and with a .acl extension. But why should NSS add extensions with a POST? What is the point of demanding that with content-type specified as text/turtle foo becomes foo.ttl and foo.ttl becomes foo.ttl.ttl and foo.acl becomes foo.acl.ttl? None of those make sense to me but maybe I'm missing something.

@RubenVerborgh

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

But why should NSS add extensions with a POST?

It shouldn't; but POST shouldn't be used to create ACL resources, as the resource URL is unpredictable (and needs to be predictable for ACL).

@jeff-zucker

This comment has been minimized.

Copy link
Author

commented Jul 8, 2019

Hmm, thanks, that is a great explanation of why PUT is needed in that case. It hadn't occurred to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.