Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBG-3699: [3.1.4 backport] Error from handleChangesResponse not handled correctly #6670

Merged
merged 3 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion base/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down
3 changes: 2 additions & 1 deletion db/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"context"
"errors"
"fmt"
"net/http"
"strconv"
"time"

Expand All @@ -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)
Expand Down
8 changes: 2 additions & 6 deletions db/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand All @@ -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")
}
}
}
Expand Down
80 changes: 80 additions & 0 deletions rest/blip_api_crud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2832,3 +2832,83 @@ func TestBlipRefreshUser(t *testing.T) {
require.NoError(t, err)
require.NotContains(t, string(body), "Panic:")
}

func TestOnDemandImportBlipFailure(t *testing.T) {
if !base.TestUseXattrs() {
t.Skip("Test performs import, not valid for non-xattr mode")
}
base.SetUpTestLogging(t, base.LevelDebug, base.KeyHTTP, base.KeySync, base.KeySyncMsg)
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"}`
markerBody := `{"prop":true}`
_ = rt.PutDoc(docID, validBody)
btc, err := NewBlipTesterClientOptsWithRT(t, rt, &BlipTesterClientOpts{
Username: "user",
Channels: []string{"*"},
})
require.NoError(t, err)
defer btc.Close()
require.NoError(t, btc.StartOneshotPull())

output, found := btc.WaitForDoc(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.PutDoc(markerDoc, markerBody)

rt.GetSingleTestDatabaseCollection().FlushRevisionCacheForTest()

btc2, err := NewBlipTesterClientOptsWithRT(t, rt, &BlipTesterClientOpts{
Username: "user",
Channels: []string{"*"},
})
require.NoError(t, err)
defer btc2.Close()

require.NoError(t, btc.StartOneshotPull())

btc.WaitForDoc(markerDoc)
})
}
}
2 changes: 1 addition & 1 deletion rest/importtest/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading