diff --git a/base/error.go b/base/error.go index 2659525a72..41af94cbe4 100644 --- a/base/error.go +++ b/base/error.go @@ -42,7 +42,7 @@ var ( ErrAlreadyExists = &sgError{"Already exists"} ErrNotFound = &sgError{"Not Found"} ErrUpdateCancel = &sgError{"Cancel update"} - ErrImportCancelledPurged = &sgError{"Import Cancelled Due to Purge"} + ErrImportCancelledPurged = HTTPErrorf(http.StatusNotFound, "Import Cancelled Due to Purge") ErrChannelFeed = &sgError{"Error while building channel feed"} ErrXattrNotFound = &sgError{"Xattr Not Found"} ErrTimeout = &sgError{"Operation timed out"} diff --git a/db/import.go b/db/import.go index 927b99ad20..7d99f21be0 100644 --- a/db/import.go +++ b/db/import.go @@ -14,6 +14,7 @@ import ( "context" "errors" "fmt" + "net/http" "strconv" "time" @@ -40,7 +41,7 @@ func (db *DatabaseCollectionWithUser) ImportDocRaw(ctx context.Context, docid st err := body.Unmarshal(value) if err != nil { base.InfofCtx(ctx, base.KeyImport, "Unmarshal error during importDoc %v", err) - return nil, err + return nil, base.HTTPErrorf(http.StatusNotFound, "Error unmarshalling %s: %s", base.UD(docid).Redact(), err) } err = validateImportBody(body) diff --git a/db/validation.go b/db/validation.go index de64211166..c18af65793 100644 --- a/db/validation.go +++ b/db/validation.go @@ -51,10 +51,6 @@ func validateAPIDocUpdate(body Body) error { // validateImportBody validates incoming import bodies func validateImportBody(body Body) error { - if body == nil { - return base.ErrEmptyDocument - } - if isPurged, ok := body[BodyPurged].(bool); ok && isPurged { return base.ErrImportCancelledPurged } @@ -63,7 +59,7 @@ func validateImportBody(body Body) error { disallowed := []string{BodyId, BodyRev, BodyExpiry, BodyRevisions} for _, prop := range disallowed { if _, ok := body[prop]; ok { - return base.HTTPErrorf(http.StatusBadRequest, "top-level property '"+prop+"' is a reserved internal property therefore cannot be imported") + return base.HTTPErrorf(http.StatusNotFound, "top-level property '"+prop+"' is a reserved internal property therefore cannot be imported") } } // TODO: Validate attachment data to ensure user is not setting invalid attachments @@ -80,7 +76,7 @@ func validateBlipBody(ctx context.Context, rawBody []byte, doc *Document) error // Only unmarshal if raw body contains the disallowed property if bytes.Contains(rawBody, []byte(`"`+prop+`"`)) { if _, ok := doc.Body(ctx)[prop]; ok { - return base.HTTPErrorf(http.StatusBadRequest, "top-level property '"+prop+"' is a reserved internal property") + return base.HTTPErrorf(http.StatusNotFound, "top-level property '"+prop+"' is a reserved internal property") } } } diff --git a/rest/blip_api_crud_test.go b/rest/blip_api_crud_test.go index a8de25238e..213865195f 100644 --- a/rest/blip_api_crud_test.go +++ b/rest/blip_api_crud_test.go @@ -2832,3 +2832,82 @@ func TestBlipRefreshUser(t *testing.T) { require.NoError(t, err) require.NotContains(t, string(body), "Panic:") } + +func TestOnDemandImportBlipFailure(t *testing.T) { + base.SetUpTestLogging(t, base.LevelDebug, base.KeyHTTP, base.KeySync, base.KeySyncMsg) + btcRunner := NewBlipTesterClientRunner(t) + btcRunner.Run(func(t *testing.T, SupportedBLIPProtocols []string) { + rt := NewRestTester(t, &RestTesterConfig{PersistentConfig: true, GuestEnabled: true}) + defer rt.Close() + config := rt.NewDbConfig() + config.AutoImport = false + RequireStatus(t, rt.CreateDatabase("db", config), http.StatusCreated) + testCases := []struct { + name string + invalidBody []byte + }{ + { + name: "_id property", + invalidBody: []byte(`{"_id": "doc1"}`), + }, + { + name: "_exp property", + invalidBody: []byte(`{"_exp": 1}`), + }, + { + name: "_rev property", + invalidBody: []byte(`{"_rev": "abc1"}`), + }, + { + name: "_revisions property", + invalidBody: []byte(`{"_revisions": {"start": 0, "ids": ["foo", "def]"}}`), + }, + + { + name: "_purged property", + invalidBody: []byte(`{"_purged": true}`), + }, + { + name: "invalid json", + invalidBody: []byte(``), + }, + } + for i, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + docID := fmt.Sprintf("doc%d,", i) + markerDoc := fmt.Sprintf("markerDoc%d", i) + validBody := `{"foo":"bar"}` + _ = rt.PutDoc(docID, validBody) + btc := btcRunner.NewBlipTesterClientOptsWithRT(rt, &BlipTesterClientOpts{ + Username: "user", + Channels: []string{"*"}, + SupportedBLIPProtocols: SupportedBLIPProtocols, + }) + defer btc.Close() + require.NoError(t, btcRunner.StartOneshotPull(btc.id)) + + output, found := btcRunner.WaitForDoc(btc.id, docID) + require.True(t, found) + require.JSONEq(t, validBody, string(output)) + + err := rt.GetSingleDataStore().SetRaw(docID, 0, nil, testCase.invalidBody) + require.NoError(t, err) + + rt.CreateTestDoc(markerDoc) + + rt.GetSingleTestDatabaseCollection().FlushRevisionCacheForTest() + + btc2 := btcRunner.NewBlipTesterClientOptsWithRT(rt, &BlipTesterClientOpts{ + Username: "user", + Channels: []string{"*"}, + SupportedBLIPProtocols: SupportedBLIPProtocols, + }) + defer btc2.Close() + + require.NoError(t, btcRunner.StartOneshotPull(btc2.id)) + + btcRunner.WaitForDoc(btc2.id, markerDoc) + }) + } + }) +} diff --git a/rest/importtest/import_test.go b/rest/importtest/import_test.go index d19af2b2a8..2c0d4efefa 100644 --- a/rest/importtest/import_test.go +++ b/rest/importtest/import_test.go @@ -2301,7 +2301,7 @@ func TestImportInternalPropertiesHandling(t *testing.T) { name: "Valid _id", importBody: map[string]interface{}{"_id": "documentid"}, expectReject: true, - expectedStatusCode: base.IntPtr(400), + expectedStatusCode: base.IntPtr(http.StatusNotFound), }, { name: "Valid _rev",