Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
In the very latest code, the REST Server is allowing access to disabled resources. Patch to follow.
Comments
Comment #1
jamsilver CreditAttribution: jamsilver commentedpatch attached
Comment #3
ygerasimov CreditAttribution: ygerasimov commentedoh! this is really nasty bug. lets have test to make sure it will never happen again.
Comment #4
mradcliffeAdded a test class to test a disabled resource (I chose the system resource). Is this the approach you want to take for the test, ygerasimov?
Comment #6
mradcliffeFor some reason the commit I made (2nd patch) didn't include changes to files, just the added test, so it didn't get run. The last patch demonstrates it completely though (the git apply one). Sorry.
Comment #7
kylebrowning CreditAttribution: kylebrowning commentedUgh, why are tests failing, we need to fix those issues if they are failing in their current state. Ill take a look at this tomorrow.
Comment #8
mradcliffe#4: services-1693342-04.patch queued for re-testing.
Comment #10
mradcliffeLooking into the other tests.
Comment #11
mradcliffesaveNewEndpoint doesn't add the target_actions array to the endpoint, which was introduced in #1484992: Attach file to node targeted_action. There were tests added in that patch, which makes things fail. After I fixed that, the tests still fail because on my system tests are changin node ids? Furthermore it looks like that issue has been reset to 7.x-3.x branch for further development
Here's a patch with #1694438: Aliased resources are inaccessible applied as well just to see the changing test failures.
Comment #13
mradcliffeOh, nice. It was just my test system. Down to the same 6 failures that are in other issues now. :-)
Comment #14
mradcliffeSo both user resource and xml rpc tests pass locally for me. I'm going to set this back to needs review for now.
Here are the formatted patches that I applied (note the one from #1694438: Aliased resources are inaccessible).
Comment #15
kylebrowning CreditAttribution: kylebrowning commentedYeah unfortunately the tests dont pass at all with these patches.
Comment #16
mradcliffeEdit: Which tests? The same 6 tests that fail on every single issue?
#11: 1693342-allow-access-disabled-resources-with-1694438.patch queued for re-testing.
Comment #17
kylebrowning CreditAttribution: kylebrowning commentedNah its way more than 6, Comment tests fails, file upload tests fail and it actually has a 500 error.
Comment #18
kylebrowning CreditAttribution: kylebrowning commentedPatch #11 might be good, but the patches #14 do not.
Comment #19
mradcliffeHmm, weird. I'll have to look at my local repo again.
Comment #20
mradcliffeAccidentally changed status.
Comment #21
mradcliffeI cloned a clean services 7.x-3.x branch, took the combined patch in #11 and git apply'd it. Then checkout a clean services 7.x-3.x branch, applied the patches in #14 and git am'd them. Then git diff'd the two branches, and there were no differences. I'm confused now. I think there might be some issue with the way the patches in #14 were applied. Not sure what happend @kylebrowning.
Comment #22
kylebrowning CreditAttribution: kylebrowning commentedok ill do the same.
Comment #23
mradcliffe#11: 1693342-allow-access-disabled-resources-with-1694438.patch queued for re-testing.
Comment #25
mradcliffePulled out #1694438: Aliased resources are inaccessible from the patch. Setting to review (with do-not-test) until that issue is committed.
Comment #26
kylebrowning CreditAttribution: kylebrowning commented#11: 1693342-allow-access-disabled-resources-with-1694438.patch queued for re-testing.
Comment #27
kylebrowning CreditAttribution: kylebrowning commentedI fixed the tests, lets see what happens!
Comment #28
kylebrowning CreditAttribution: kylebrowning commentedWell look at that! Committed and fixed in dev!