test/cli: content blocking tests for CARs #10359
Conversation
| for _, format := range []string{"", "raw", "car"} { // CARs should also fail immediately since we access the file directly. | ||
| // Confirm that denylist is active for every content path in 'testCases' with different formats | ||
| gwTestName := fmt.Sprintf("Gateway denies %s with format %s", testCase.name, format) | ||
| t.Run(gwTestName, func(t *testing.T) { | ||
| resp := client.Get(testCase.path + "?format=" + format) | ||
| assert.Equal(t, http.StatusGone, resp.StatusCode, statusExpl) | ||
| assert.Contains(t, resp.Body, blockedMsg, bodyExpl) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The CAR times out on both 0.26 and current PR in some tests, such as Double hash (blake3) Path block under blake3 root CID. This is because we don't have the data locally, and it seems that the node tries to fetch it.
This test returns 500 for the Libp2p gateway since it is an offline gateway.
There was a problem hiding this comment.
It feels like some tests fail due to the same bug as #10361
test/cli/content_blocking_test.go
Outdated
| // Does the gateway deny indirectly blocked CID? | ||
| t.Run("Gateway Denies indirectly blocked CID", func(t *testing.T) { | ||
| t.Parallel() | ||
| resp := client.Get("/ipfs/" + blockedCIDInParentDirCID) | ||
| assert.Equal(t, http.StatusGone, resp.StatusCode, statusExpl) | ||
| assert.NotEqual(t, "directly blocked file content", resp.Body) | ||
| assert.Contains(t, resp.Body, blockedMsg, bodyExpl) | ||
| }) | ||
|
|
||
| // Does the gateway return CAR | ||
| t.Run("Gateway returns parent path CAR without blocked CID", func(t *testing.T) { | ||
| resp := client.Get("/ipfs/" + parentDirCID + "?format=car") | ||
| require.Equal(t, http.StatusOK, resp.StatusCode) | ||
|
|
||
| bs, err := cbs.NewReadOnly(strings.NewReader(resp.Body), nil) | ||
| require.NoError(t, err) | ||
|
|
||
| has, err := bs.Has(context.Background(), cid.MustParse(blockedCIDInParentDirCID)) | ||
| require.NoError(t, err) | ||
| require.False(t, has) | ||
| }) |
There was a problem hiding this comment.
Fail on both 0.26 and master.
There was a problem hiding this comment.
Filled #10361, let's continue there
| t.Run("Denies Blocked CID (CAR)", func(t *testing.T) { | ||
| t.Parallel() | ||
| resp, err := libp2pClient.Get(fmt.Sprintf("/ipfs/%s?format=car", blockedCID)) | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
| assert.Equal(t, http.StatusGone, resp.StatusCode, statusExpl) | ||
| body, err := io.ReadAll(resp.Body) | ||
| require.NoError(t, err) | ||
| assert.NotEqual(t, string(body), "directly blocked file content") | ||
| assert.Contains(t, string(body), blockedMsg, bodyExpl) | ||
| }) |
There was a problem hiding this comment.
Passes on both 0.26 and master.
hacdias
force-pushed
the
tests-blocking
branch
from
02d4d07
to
2ab7aee
Compare
|
@lidel I don't think any behaviour is different since 0.26. Could you double check? |
There was a problem hiding this comment.
I also was not able to find a case which looks like a regression since 0.26.
Bit late on my end, but iiuc the bug described in #10361 was always present.
I propose we:
- proceed with Release 0.27 #10306 (RC2)
- close this PR draft – it served its purpose (comparing with 0.26 to identify regressions)
- merge cleanup from test: cleanup content blocking tests #10360 instead
- fix denylist:
?format=cardoes not skip directly blocked CIDs #10361 in next iteration (Kubo 0.28)
| t.Parallel() | ||
| resp := client.Get("/ipfs/" + blockedCIDInParentDirCID) | ||
| assert.Equal(t, http.StatusGone, resp.StatusCode, statusExpl) | ||
| assert.NotEqual(t, "directly blocked file content", resp.Body) |
There was a problem hiding this comment.
nit: does not matter much but this is a dir, not a file, so body will never match :-)
| assert.NotEqual(t, "directly blocked file content", resp.Body) |
| @@ -36,6 +38,7 @@ func TestContentBlocking(t *testing.T) { | |||
| // Create CIDs we use in test | |||
| h.WriteFile("blocked-dir/subdir/indirectly-blocked-file.txt", "indirectly blocked file content") | |||
| parentDirCID := node.IPFS("add", "--raw-leaves", "-Q", "-r", filepath.Join(h.Dir, "blocked-dir")).Stdout.Trimmed() | |||
| blockedCIDInParentDirCID := node.IPFS("add", "--raw-leaves", "-Q", "-r", filepath.Join(h.Dir, "blocked-dir", "subdir")).Stdout.Trimmed() | |||
There was a problem hiding this comment.
💭 During review, I've realized some things changed in refactors and dir names are confusing: blocked-dir is not blocked 🙃 and subdir is directly blocked, I fixed names in #10360 so its easier to reason about test file in the future.
| // Does the gateway return CAR | ||
| t.Run("Gateway returns parent path CAR without blocked CID", func(t *testing.T) { | ||
| resp := client.Get("/ipfs/" + parentDirCID + "?format=car") | ||
| assert.Equal(t, http.StatusOK, resp.StatusCode) | ||
|
|
||
| bs, err := cbs.NewReadOnly(strings.NewReader(resp.Body), nil) | ||
| assert.NoError(t, err) | ||
|
|
||
| has, err := bs.Has(context.Background(), cid.MustParse(blockedCIDInParentDirCID)) | ||
| assert.NoError(t, err) | ||
| assert.False(t, has) | ||
| }) |
There was a problem hiding this comment.
💭 iiuc this does not pass because /ipfs/parentDirCID/subdir here is blocked by a subpath rule, and not by a CID rule (blockedCIDInParentDirCID). It was this way since the beginning, so not a regression blocking 0.27, but a bug which we should fix in future release.
I've added a version of this test that uses path and commented out one which does not pass yet to #10360
No description provided.