Currently you can create entities without bundles, which will break the overview of that entity type.
We should restrict that somehow.

CommentFileSizeAuthor
#3 fix_no_bundles2.patch2.93 KBsepgil
#1 fix_no_bundles.patch2.8 KBsepgil

Comments

sepgil’s picture

sepgil commented
Status: Active » Needs review
StatusFileSize
new fix_no_bundles.patch2.8 KB

I made a patch, which fixes this issue and also added a test to test the new functionality. However I'm not sure if I'm returning the right HTTP status code(currently 406 Not Acceptable). Would it be better to use 400 Bad Request.
Btw: I also fixed 2 spelling errors I found in some comments.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work 
+++ b/restws.entity.inc
@@ -118,6 +118,15 @@ class RestWSEntityResourceController implements RestWSResourceControllerInterfac
+        if (!array_key_exists($bundle_key, $values))
+          throw new RestWSException('Missing bundle: ' . $bundle_key, 406);

inline if statements are not allowed, always use "{}"

+++ b/restws.test
@@ -80,6 +80,22 @@ class RestWSTestCase extends DrupalWebTestCase {
   /**
+   * Tests malicious requests.
+   */

That comment does not really fit, shouldn't it be "Test bad creation request with missing bundle key"?

+++ b/restws.test
@@ -80,6 +80,22 @@ class RestWSTestCase extends DrupalWebTestCase {
+  function testMaliciousRequests() {

I think you should also check that loading the node by the title does not return any result (after the request).

I'm not sure about 400 vs. 406. 400 says that the syntax is malformed, which is not the case here. On the other hand 406 says that the request does not conform to the accept headers, so this seems wrong, too.

sepgil’s picture

sepgil commented
Status: Needs work » Needs review
StatusFileSize
new fix_no_bundles2.patch2.93 KB

That comment does not really fit, shouldn't it be "Test bad creation request with missing bundle key"?

I wanted a general comment & title, in case we want to add more tests to this function. I renamed it to testBadRequests.

I've fixed the other 2 issues to in the new patch.

I'm not sure about 400 vs. 406. 400 says that the syntax is malformed, which is not the case here. On the other hand 406 says that the request does not conform to the accept headers, so this seems wrong, too.

There seems to be no appropriate status code for it, so maybe should keep 406 for now...

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Fixed

Committed, thanks! http://drupalcode.org/project/restws.git/commit/2318d17

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.