Prevent infinite relationship nesting loop

[spawnia]

All is fine if i make such a request:

this.api.get('printers', {
      include: 'paper',
    })

This works as well:

this.api.get('printers', {
      include: 'labels',
    })

And this returns 400 Bad Request:

this.api.get('printers', {
      include: 'labels,nonexistinginclude',
    })

But this causes to client to freeze, the Browsertab becomes unresponsive and i have to close it:

this.api.get('printers', {
      include: 'paper,labels',
    })

I tried sending the request from elsewhere, no problem whatsoever. Maybe Kitsu can not deal with multiple includes?

[wopian]

Are you able to provide the response it's receiving? I can't replicate this issue in the demo on codepen: https://codepen.io/wopian/pen/RxmEeK

[spawnia]

I put up a Mock API to test on: http://demo9418135.mockable.io/printers?include=paper,labels

I can not get codepen to call that, i tried it like this:

const printerApi = new Kitsu({
   baseURL: 'http://demo9418135.mockable.io/',
})

printerApi.get('printers', {include: 'paper,labels'})
   .then(res => {
      const pre = document.getElementsByTagName('pre')[0];
      pre.innerHTML = JSON.stringify(res, null, 2)
   })

The problem may be related to using a plural include (labels) and a singular include (paper) in the same request. paper actually refers to a resource with the type papers.

[wopian]

You'd need to give it the HTTPS address (https://demo9418135.mockable.io/printers?include=paper,labels) for the request to be sent.

---

The response you've provided has the included papers and labels also include their own relationships. Which in this case refers to the root printers and the included papers and labels.

While this is a valid JSON:API structure, it is not a valid response for the query provided. The include=paper,labels query should only have printers' relationships resolved. The included paper and labels relationships should just contain the links (below).

{
  relationships: {
    paper: {
      links: { self: '', related: '' }
    }
  }
}

include=paper.labels,labels.paper,labels.printers would be a request that should genuinely run into this bug.

This is causing kitsu to infinitely nest the relationships into the parent printers resource, i.e:

{
  data: {
    type: 'printers',
    paper: {
      labels: [
        paper: { labels [ /* infinitely nest papers and printers */ ] },
        printers: [ { /* infinitely nest root data structure again */ } ]
      ],
    },
    labels: [
      paper: { labels [ /* infinitely nest papers and printers */ ] },
      printers: [ { /* infinitely nest root data structure again */ } ]
    ]
  }
}

Which quite quickly locks up the runtime in what's basically a DOS attack. Less of an issue in browsers as they can detect and kill the process when this happens. In node it killed my distro when added to jest's tests.

[nanawel]

Hi there,
is this bug being working on? I have a similar situation where relationships can sometimes create loops and it breaks the browser when the page loads (FF and Chromium).
I don't know how to fix this on the server-side because basically the structure is valid and really reflects the relationships between my entities.

[wopian]

Not being actively looked into at the moment. PRs are welcome in the meantime however.

Possible solutions

1. Limit nesting depth. Nerfs the usability for deeply nested relationships that aren't infinitely looping.
2. Abort when the the same relationship object is added in a relationship chain (e.g user 1 -> libraryEntry 1 -> user 1
3. Additional parameter to Kitsu class and/or per-request to keep included relationships inside the included array.
   • Fixes the issue for those aware of the parameter
   • Package will still explode if an unaware user requests a resource that causes infinite nesting.
4. Keep included relationships in included array and link to the relationships using Object getters in data and included[].relationshipName.data[]? with included made non-enumerable.
   • However I don't know if this would still run into the infinite relationship nesting.

[fspoettel]

Is this still an issue now after #601 landed?

[wopian]

Is this still an issue now after #601 landed?

Nope, #601 should have fixed this issue and appears to have done in the circular relationship tests I've written