Skip to content

Commit bb32c63

Browse files
author
Trisha Gee
committed
Added validation to check a Document passed into FindAndReplace does not contain update operators
1 parent 0750582 commit bb32c63

5 files changed

Lines changed: 64 additions & 22 deletions

File tree

‎driver/src/main/org/mongodb/operation/CommandResultWithPayloadDecoder.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,14 @@ public Document decode(final BSONReader reader) {
4343
reader.readStartDocument();
4444
while (reader.readBSONType() != END_OF_DOCUMENT) {
4545
final String fieldName = reader.readName();
46-
final Object result;
4746
final BSONType bsonType = reader.getCurrentBSONType();
4847
if (bsonType.equals(DOCUMENT) && fieldName.equals(FIELD_CONTAINING_PAYLOAD)) {
49-
result = payloadDecoder.decode(reader);
48+
document.put(fieldName, payloadDecoder.decode(reader));
5049
} else {
51-
result = codecs.decode(reader);
50+
document.put(fieldName, codecs.decode(reader));
5251
}
53-
document.put(fieldName, result);
5452
}
55-
5653
reader.readEndDocument();
57-
5854
return document;
5955
}
6056
}

‎driver/src/main/org/mongodb/operation/CommandWithPayloadEncoder.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import org.mongodb.Document;
2121
import org.mongodb.Encoder;
2222
import org.mongodb.codecs.Codecs;
23-
import org.mongodb.codecs.validators.FieldNameValidator;
23+
import org.mongodb.codecs.validators.Validator;
2424

2525
import java.util.Map;
2626

@@ -29,12 +29,14 @@ class CommandWithPayloadEncoder<T> implements Encoder<Document> {
2929
private final Codecs codecs = Codecs.createDefault();
3030

3131
private final Encoder<T> payloadEncoder;
32-
private final FieldNameValidator fieldNameValidator = new FieldNameValidator();
3332
private final String fieldContainingPayload;
33+
private final Validator<T> payloadValidator;
3434

35-
CommandWithPayloadEncoder(final String fieldContainingPayload, final Encoder<T> payloadEncoder) {
35+
CommandWithPayloadEncoder(final String fieldContainingPayload, final Encoder<T> payloadEncoder,
36+
final Validator<T> payloadValidator) {
3637
this.payloadEncoder = payloadEncoder;
3738
this.fieldContainingPayload = fieldContainingPayload;
39+
this.payloadValidator = payloadValidator;
3840
}
3941

4042
//we need to cast the payload to (T) to encode it
@@ -45,11 +47,12 @@ public void encode(final BSONWriter bsonWriter, final Document value) {
4547

4648
for (final Map.Entry<String, Object> entry : value.entrySet()) {
4749
final String fieldName = entry.getKey();
48-
fieldNameValidator.validate(fieldName);
4950

5051
bsonWriter.writeName(fieldName);
5152
if (fieldContainingPayload.equals(fieldName)) {
52-
payloadEncoder.encode(bsonWriter, (T) entry.getValue());
53+
final T payload = (T) entry.getValue();
54+
payloadValidator.validate(payload);
55+
payloadEncoder.encode(bsonWriter, payload);
5356
} else {
5457
codecs.encode(bsonWriter, entry.getValue());
5558
}

‎driver/src/main/org/mongodb/operation/FindAndReplaceOperation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public FindAndReplaceOperation(final BufferProvider bufferProvider, final Sessio
3939
this.namespace = namespace;
4040
this.findAndReplace = findAndReplace;
4141
resultDecoder = new CommandResultWithPayloadDecoder<T>(payloadDecoder);
42-
commandEncoder = new CommandWithPayloadEncoder<T>("update", payloadEncoder);
42+
commandEncoder = new CommandWithPayloadEncoder<T>("update", payloadEncoder, new FindAndReplaceValidator<T>());
4343
}
4444

4545
@SuppressWarnings("unchecked")

‎driver/src/main/org/mongodb/operation/FindAndReplaceValidator.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright (c) 2008 - 2013 10gen, Inc. <http://10gen.com>
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.mongodb.operation;
18+
19+
import org.mongodb.Document;
20+
import org.mongodb.codecs.validators.Validator;
21+
22+
import static java.lang.String.format;
23+
24+
public class FindAndReplaceValidator<T> implements Validator<T> {
25+
@Override
26+
public void validate(final T value) {
27+
if (value instanceof Document){
28+
for (String key : ((Document) value).keySet()) {
29+
if (key.startsWith("$")) {
30+
throw new IllegalArgumentException(format("Can't use update operators (beginning with '$') in a find and replace "
31+
+ "operation (Bad Key: '%s')", value));
32+
}
33+
}
34+
}
35+
}
36+
}

‎driver/src/test/acceptance/org/mongodb/acceptancetest/crud/FindAndReplaceAcceptanceTest.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@
2626

2727
import java.util.Date;
2828

29+
import static org.hamcrest.CoreMatchers.startsWith;
2930
import static org.hamcrest.core.Is.is;
3031
import static org.hamcrest.core.IsEqual.equalTo;
3132
import static org.junit.Assert.assertNull;
3233
import static org.junit.Assert.assertThat;
34+
import static org.junit.Assert.fail;
3335

3436
/**
3537
* Documents and tests the functionality provided for find-and-replace atomic operations.
@@ -146,16 +148,21 @@ public void shouldInsertDocumentWhenFilterDoesNotMatchAnyDocumentsAndUpsertFlagI
146148
document, equalTo(replacementDocument));
147149
}
148150

149-
// @Test(expected = IllegalArgumentException.class)
150-
// public void shouldThrowAnExceptionIfReplacementContainsUpdateOperators() {
151-
// final Document documentInserted = new Document(KEY, VALUE_TO_CARE_ABOUT);
152-
// collection.insert(documentInserted);
153-
//
154-
// assertThat(collection.count(), is(1L));
155-
//
156-
// collection.filter(new Document(KEY, VALUE_TO_CARE_ABOUT))
157-
// .replaceAndGet(new Document("$set", new Document("foo", "bar")), Get.AfterChangeApplied);
158-
// }
151+
@Test
152+
public void shouldThrowAnExceptionIfReplacementContainsUpdateOperators() {
153+
final Document documentInserted = new Document(KEY, VALUE_TO_CARE_ABOUT);
154+
collection.insert(documentInserted);
155+
156+
assertThat(collection.find().count(), is(1L));
157+
158+
try {
159+
collection.find(new Document(KEY, VALUE_TO_CARE_ABOUT))
160+
.getOneAndReplace(new Document("$inc", new Document("someNumber", "635")));
161+
fail("Should throw an IllegalArgumentException when trying to do a replace with a document containing an update operation");
162+
} catch (IllegalArgumentException e) {
163+
assertThat(e.getMessage(), startsWith("Can't use update operators (beginning with '$') in a find and replace operation"));
164+
}
165+
}
159166

160167
//TODO: should not be able to change the ID of a document
161168

0 commit comments

Comments
 (0)