Skip to content

Unexpected 409 response on PATCH #1226

Closed

Description

@michielbdejong
  • open a private browsing window
  • visit https://generator.inrupt.com/
  • log in with either solid.community (which is on NSS version 4) or inrupt.net (which is on NSS version 5)
  • edit something on your profile (for instance your name or your role)
  • refresh the page

if you logged in with solid.community, you'll see the change was successful
if you logged in with inrupt.net you'll see the change failed.

In the developer tools you'll see that the PATCH request was responded to with a 409 status, which doesn't make a lot of sense.

Activity

  1. jaxoncreed commented on Jun 12, 2019

    @jaxoncreed
    Contributor

    Testing now

  2. james-martin-jd commented on Jun 12, 2019

    @james-martin-jd

    In the past I've received 409 errors when the value/node to delete during a PATCH can't be found. If I recall, some of the file names changed recently, is it possible the PATCH can't find the file with the delete and so it's throwing 409?

  3. jaxoncreed commented on Jun 12, 2019

    @jaxoncreed
    Contributor

    @RubenVerborgh In reference to 111e63b#diff-93af968f3e26d51a3f13dc503ce4b347R69

    this._rootpath is /Users/m/inrupt/node-solid-server/data and the file location is /Users/m/inrupt/node-solid-server/data/jackson.localhost/profile/card$.ttl

    This means that when that line of code runs, there's still a "jackson.localhost" before profile/card which is where that's coming from. Is this how it should work?

  4. jaxoncreed commented on Jun 12, 2019

    @jaxoncreed
    Contributor

    For more context when it's 409ing it creates the error The patch could not be applied. Could not find to delete: <https://jackson.localhost:8443/profile/card#me> <http://xmlns.com/foaf/0.1/name> "jackson"

  5. RubenVerborgh commented on Jun 12, 2019

    @RubenVerborgh
    Contributor

    @jaxoncreed So what you mean is that, when calling mapFileToUrl with { path: '/Users/m/inrupt/node-solid-server/data/jackson.localhost/profile/card$.ttl', hostname: 'jackson.localhost' }, it gives back https://jackson.localhost:8443/jackson.localhost/profile/card? (took that from somewhere else)

    That would be incorrect behavior indeed.

    Weird that this never popped up; corresponding tests are wrong (https://github.com/solid/node-solid-server/blob/b7b5bffd1b94f4976c04c3cf845854efecc81168/test/unit/resource-mapper-test.js#L536-L545) and should be adjusted, then the code can be fixed.

    Please add me as reviewer to the PR on this one.

  6. jaxoncreed commented on Jun 12, 2019

    @jaxoncreed
    Contributor

    Okay will do

  7. RubenVerborgh commented on Jun 12, 2019

    @RubenVerborgh
    Contributor

    @jaxoncreed So here's what you probably want to do:

    • The current startsWith check makes the whole thing work with relative or absolute URLs. So let's keep that one in.
    • After that, if this._includeHost is true, the remaining path MUST start with /HOSTNAME/.
      • If not, throw an Error; that path is invalid. (Unit tests should check this.)
      • If yes, strip it off and continue.
  8. jaxoncreed commented on Jun 12, 2019

    @jaxoncreed
    Contributor

    Ah good thing you caught me before I modified "startsWith"

  9. michielbdejong commented on Jun 17, 2019

    @michielbdejong
    MemberAuthor

    I think this issue affected all PATCH requests, so it should have been detected by the test-suite. Will see if we can run the test-suite in Travis so we can use that information when reviewing PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions