Skip to content

Commit

Permalink
Added optional id validation to prevent documents with invalid chars …
Browse files Browse the repository at this point in the history 
…in id property to be created (#41108)

* Added optional id validation to prevent documents with invalid chars in id property to be created
  • Loading branch information
@FabianMeiswinkel
FabianMeiswinkel authored Jul 12, 2024
1 parent 73d1524 commit b2be526
Show file tree
Hide file tree
Showing 14 changed files with 268 additions and 113 deletions.
5 changes: 5 additions & 0 deletions ...ryption/src/main/java/com/azure/cosmos/encryption/implementation/EncryptionProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,11 @@ public Mono<byte[]> encrypt(byte[] payload) {
}

public Mono<byte[]> encrypt(JsonNode itemJObj) {

if (itemJObj != null) {
Utils.validateIdValue(itemJObj.get(Constants.PROPERTY_NAME_ID));
}

return encryptObjectNode(itemJObj).map(
encryptedObjectNode -> EncryptionUtils.serializeJsonToByteArray(
CosmosItemSerializer.DEFAULT_SERIALIZER,
Expand Down
2 changes: 1 addition & 1 deletion ...-encryption/src/main/java/com/azure/cosmos/encryption/implementation/EncryptionUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class EncryptionUtils {
}

public static byte[] serializeJsonToByteArray(CosmosItemSerializer itemSerializer, Object object) {
return toByteArray(Utils.serializeJsonToByteBuffer(itemSerializer, object, null));
return toByteArray(Utils.serializeJsonToByteBuffer(itemSerializer, object, null, false));
}

public static ObjectMapper getSimpleObjectMapper() {
Expand Down
223 changes: 138 additions & 85 deletions sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosItemIdEncodingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package com.azure.cosmos;

import com.azure.cosmos.implementation.Configs;
import com.azure.cosmos.implementation.HttpConstants;
import com.azure.cosmos.implementation.Utils;
import com.azure.cosmos.models.CosmosContainerProperties;
Expand All @@ -22,7 +23,6 @@
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Factory;
import org.testng.annotations.Ignore;
import org.testng.annotations.Test;
import reactor.core.Exceptions;

Expand Down Expand Up @@ -437,7 +437,6 @@ public void idWithDisallowedCharQuestionMark() {
this.executeTestCase(scenario);
}

@Ignore("Throws IllegalArgumentException instead of CosmosException")
@Test(groups = { "fast", "emulator" }, timeOut = TIMEOUT)
public void idWithDisallowedCharForwardSlash() {
TestScenario scenario = new TestScenario(
Expand All @@ -446,21 +445,49 @@ public void idWithDisallowedCharForwardSlash() {
new TestScenarioExpectations(
ConnectionMode.GATEWAY.toString(),
HttpConstants.StatusCodes.CREATED,
HttpConstants.StatusCodes.NOTFOUND,
HttpConstants.StatusCodes.NOTFOUND,
HttpConstants.StatusCodes.NOTFOUND),
-1 , // Non-CosmosException - in this case IllegalArgumentException
-1,
-1),
new TestScenarioExpectations(
"COMPUTE_GATEWAY",
HttpConstants.StatusCodes.CREATED,
HttpConstants.StatusCodes.NOTFOUND,
HttpConstants.StatusCodes.NOTFOUND,
HttpConstants.StatusCodes.NOTFOUND),
-1 , // Non-CosmosException - in this case IllegalArgumentException
-1,
-1),
new TestScenarioExpectations(
ConnectionMode.DIRECT.toString(),
HttpConstants.StatusCodes.CREATED,
HttpConstants.StatusCodes.NOTFOUND,
HttpConstants.StatusCodes.NOTFOUND,
HttpConstants.StatusCodes.NOTFOUND));
-1 , // Non-CosmosException - in this case IllegalArgumentException
-1,
-1));

this.executeTestCase(scenario);
}

@Test(groups = { "fast", "emulator" }, timeOut = TIMEOUT)
public void idWithDisallowedCharForwardSlashButIdValidationEnabled() {
TestScenario scenario = new TestScenario(
"IdWithDisallowedCharForwardSlashButIdValidationEnabled",
"Disallowed/Chars" + UUID.randomUUID(),
true,
new TestScenarioExpectations(
ConnectionMode.GATEWAY.toString(),
HttpConstants.StatusCodes.BADREQUEST,
-1 , // Non-CosmosException - in this case IllegalArgumentException
-1,
-1),
new TestScenarioExpectations(
"COMPUTE_GATEWAY",
HttpConstants.StatusCodes.BADREQUEST,
-1 , // Non-CosmosException - in this case IllegalArgumentException
-1,
-1),
new TestScenarioExpectations(
ConnectionMode.DIRECT.toString(),
HttpConstants.StatusCodes.BADREQUEST,
-1 , // Non-CosmosException - in this case IllegalArgumentException
-1,
-1));

this.executeTestCase(scenario);
}
Expand Down Expand Up @@ -608,90 +635,102 @@ private void executeTestCase(TestScenario scenario) {

logger.info("Scenario: {}, Id: \"{}\"", scenario.name, scenario.id);

try {
CosmosItemResponse<ObjectNode> response = this.container.createItem(
getDocumentDefinition(scenario.id),
new PartitionKey(scenario.id),
null);

deserializeAndValidatePayload(response, scenario.id, expected.ExpectedCreateStatusCode);
} catch (Throwable throwable) {
CosmosException cosmosError = Utils.as(Exceptions.unwrap(throwable), CosmosException.class);
if (cosmosError == null) {
Fail.fail(
"Unexpected exception type " + Exceptions.unwrap(throwable).getClass().getName(),
throwable);
}

logger.error(cosmosError.toString());

assertThat(cosmosError.getStatusCode())
.isEqualTo(expected.ExpectedCreateStatusCode);

return;
if (scenario.idValidationEnabled) {
System.setProperty(Configs.PREVENT_INVALID_ID_CHARS, "false");
}

try {
CosmosItemResponse<ObjectNode> response = this.container.readItem(
scenario.id,
new PartitionKey(scenario.id),
ObjectNode.class);

deserializeAndValidatePayload(response, scenario.id, expected.ExpectedReadStatusCode);
} catch (Throwable throwable) {
CosmosException cosmosError = Utils.as(Exceptions.unwrap(throwable), CosmosException.class);
if (cosmosError == null) {
Fail.fail(
"Unexpected exception type " + Exceptions.unwrap(throwable).getClass().getName(),
throwable);
}
if (cosmosError.getStatusCode() == 0 &&
cosmosError.getCause() instanceof IllegalArgumentException &&
cosmosError.getCause().getCause() instanceof JsonParseException &&
cosmosError.getCause().getCause().toString().contains("<TITLE>Bad Request</TITLE>")) {
try {
CosmosItemResponse<ObjectNode> response = this.container.createItem(
getDocumentDefinition(scenario.id),
new PartitionKey(scenario.id),
null);

deserializeAndValidatePayload(response, scenario.id, expected.ExpectedCreateStatusCode);
} catch (Throwable throwable) {
CosmosException cosmosError = Utils.as(Exceptions.unwrap(throwable), CosmosException.class);
if (cosmosError == null) {
Fail.fail(
"Unexpected exception type " + Exceptions.unwrap(throwable).getClass().getName(),
throwable);
}

logger.error(cosmosError.toString());

assertThat(cosmosError.getStatusCode())
.isEqualTo(expected.ExpectedCreateStatusCode);

logger.info("HTML BAD REQUEST", cosmosError);
assertThat(expected.ExpectedReadStatusCode).isEqualTo(400);
return;
} else {
logger.info("BAD REQUEST", cosmosError);
assertThat(cosmosError.getStatusCode()).isEqualTo(expected.ExpectedReadStatusCode);
}
}

try {
CosmosItemResponse<ObjectNode> response = this.container.replaceItem(
getDocumentDefinition(scenario.id),
scenario.id,
new PartitionKey(scenario.id),
null);

deserializeAndValidatePayload(response, scenario.id, expected.ExpectedReplaceStatusCode);
} catch (Throwable throwable) {
CosmosException cosmosError = Utils.as(Exceptions.unwrap(throwable), CosmosException.class);
if (cosmosError == null) {
Fail.fail(
"Unexpected exception type " + Exceptions.unwrap(throwable).getClass().getName(),
throwable);
try {
CosmosItemResponse<ObjectNode> response = this.container.readItem(
scenario.id,
new PartitionKey(scenario.id),
ObjectNode.class);

deserializeAndValidatePayload(response, scenario.id, expected.ExpectedReadStatusCode);
} catch (Throwable throwable) {
CosmosException cosmosError = Utils.as(Exceptions.unwrap(throwable), CosmosException.class);
if (cosmosError == null) {
if (expected.ExpectedReadStatusCode == -1) {
return;
}

Fail.fail(
"Unexpected exception type " + Exceptions.unwrap(throwable).getClass().getName(),
throwable);
}
if (cosmosError.getStatusCode() == 0 &&
cosmosError.getCause() instanceof IllegalArgumentException &&
cosmosError.getCause().getCause() instanceof JsonParseException &&
cosmosError.getCause().getCause().toString().contains("<TITLE>Bad Request</TITLE>")) {

logger.info("HTML BAD REQUEST", cosmosError);
assertThat(expected.ExpectedReadStatusCode).isEqualTo(400);
return;
} else {
logger.info("BAD REQUEST", cosmosError);
assertThat(cosmosError.getStatusCode()).isEqualTo(expected.ExpectedReadStatusCode);
}
}
assertThat(cosmosError.getStatusCode()).isEqualTo(expected.ExpectedReplaceStatusCode);
}

try {
CosmosItemResponse<Object> response = this.container.deleteItem(
scenario.id,
new PartitionKey(scenario.id),
(CosmosItemRequestOptions)null);

assertThat(response.getStatusCode()).isEqualTo(expected.ExpectedDeleteStatusCode);
} catch (Throwable throwable) {
CosmosException cosmosError = Utils.as(Exceptions.unwrap(throwable), CosmosException.class);
if (cosmosError == null) {
Fail.fail(
"Unexpected exception type " + Exceptions.unwrap(throwable).getClass().getName(),
throwable);
try {
CosmosItemResponse<ObjectNode> response = this.container.replaceItem(
getDocumentDefinition(scenario.id),
scenario.id,
new PartitionKey(scenario.id),
null);

deserializeAndValidatePayload(response, scenario.id, expected.ExpectedReplaceStatusCode);
} catch (Throwable throwable) {
CosmosException cosmosError = Utils.as(Exceptions.unwrap(throwable), CosmosException.class);
if (cosmosError == null) {
Fail.fail(
"Unexpected exception type " + Exceptions.unwrap(throwable).getClass().getName(),
throwable);
}
assertThat(cosmosError.getStatusCode()).isEqualTo(expected.ExpectedReplaceStatusCode);
}
assertThat(cosmosError.getStatusCode()).isEqualTo(expected.ExpectedDeleteStatusCode);

try {
CosmosItemResponse<Object> response = this.container.deleteItem(
scenario.id,
new PartitionKey(scenario.id),
(CosmosItemRequestOptions) null);

assertThat(response.getStatusCode()).isEqualTo(expected.ExpectedDeleteStatusCode);
} catch (Throwable throwable) {
CosmosException cosmosError = Utils.as(Exceptions.unwrap(throwable), CosmosException.class);
if (cosmosError == null) {
Fail.fail(
"Unexpected exception type " + Exceptions.unwrap(throwable).getClass().getName(),
throwable);
}
assertThat(cosmosError.getStatusCode()).isEqualTo(expected.ExpectedDeleteStatusCode);
}
} finally {
System.clearProperty(Configs.PREVENT_INVALID_ID_CHARS);
}
}

Expand Down Expand Up @@ -757,17 +796,31 @@ public TestScenario(
TestScenarioExpectations computeGateway,
TestScenarioExpectations direct) {

this(name, id, false, gateway, computeGateway, direct);
}

public TestScenario(
String name,
String id,
boolean idValidationEnabled,
TestScenarioExpectations gateway,
TestScenarioExpectations computeGateway,
TestScenarioExpectations direct) {

this.name = name;
this.id = id;
this.gateway = gateway;
this.computeGateway = computeGateway;
this.direct = direct;
this.idValidationEnabled = idValidationEnabled;
}

public String name;

public String id;

public boolean idValidationEnabled;

public TestScenarioExpectations gateway;

public TestScenarioExpectations computeGateway;
Expand Down
12 changes: 8 additions & 4 deletions ...re-cosmos-tests/src/test/java/com/azure/cosmos/implementation/InternalObjectNodeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ public class InternalObjectNodeTest {
public void PojoWithTrackingId() throws IOException {
String expectedTrackingId = UUID.randomUUID().toString();
TestPojo pojo = new TestPojo();
ByteBuffer buffer = InternalObjectNode.serializeJsonToByteBuffer(pojo, CosmosItemSerializer.DEFAULT_SERIALIZER, expectedTrackingId);
ByteBuffer buffer = InternalObjectNode.serializeJsonToByteBuffer(
pojo, CosmosItemSerializer.DEFAULT_SERIALIZER, expectedTrackingId, false);
byte[] blob = new byte[buffer.remaining()];
buffer.get(blob);
validateTrackingId(blob, expectedTrackingId);
Expand All @@ -33,7 +34,8 @@ public void ByteArrayWithTrackingId() throws IOException {
String expectedTrackingId = UUID.randomUUID().toString();
ObjectNode objectNode = MAPPER.createObjectNode();
objectNode.put("id", "myId");
ByteBuffer buffer = InternalObjectNode.serializeJsonToByteBuffer(objectNode, CosmosItemSerializer.DEFAULT_SERIALIZER, expectedTrackingId);
ByteBuffer buffer = InternalObjectNode.serializeJsonToByteBuffer(
objectNode, CosmosItemSerializer.DEFAULT_SERIALIZER, expectedTrackingId, false);
byte[] blob = blob = new byte[buffer.remaining()];
buffer.get(blob);
validateTrackingId(blob, expectedTrackingId);
Expand All @@ -44,7 +46,8 @@ public void internalObjectNodeWithTrackingId() throws IOException {
String expectedTrackingId = UUID.randomUUID().toString();
InternalObjectNode intenalObjectNode = new InternalObjectNode();
intenalObjectNode.set("id", "myId", CosmosItemSerializer.DEFAULT_SERIALIZER);
ByteBuffer buffer = InternalObjectNode.serializeJsonToByteBuffer(intenalObjectNode, CosmosItemSerializer.DEFAULT_SERIALIZER, expectedTrackingId);
ByteBuffer buffer = InternalObjectNode.serializeJsonToByteBuffer(
intenalObjectNode, CosmosItemSerializer.DEFAULT_SERIALIZER, expectedTrackingId, false);
byte[] blob = new byte[buffer.remaining()];
buffer.get(blob);
validateTrackingId(blob, expectedTrackingId);
Expand All @@ -55,7 +58,8 @@ public void objectNodeWithTrackingId() throws IOException {
String expectedTrackingId = UUID.randomUUID().toString();
ObjectNode objectNode = MAPPER.createObjectNode();
objectNode.put("id", "myId");
ByteBuffer buffer = InternalObjectNode.serializeJsonToByteBuffer(objectNode, CosmosItemSerializer.DEFAULT_SERIALIZER, expectedTrackingId);
ByteBuffer buffer = InternalObjectNode.serializeJsonToByteBuffer(
objectNode, CosmosItemSerializer.DEFAULT_SERIALIZER, expectedTrackingId, false);
byte[] blob = new byte[buffer.remaining()];
buffer.get(blob);
validateTrackingId(blob, expectedTrackingId);
Expand Down
1 change: 1 addition & 0 deletions sdk/cosmos/azure-cosmos/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### 4.63.0-beta.1 (Unreleased)

#### Features Added
* Added optional id validation to prevent documents with invalid char '/' in id property to be created. - See [PR 41108](https://github.com/Azure/azure-sdk-for-java/pull/41108)
* Added support for specifying a set of custom diagnostic correlation ids in the request options. - See [PR 40835](https://github.com/Azure/azure-sdk-for-java/pull/40835)

#### Breaking Changes
Expand Down
Loading

0 comments on commit b2be526

Please sign in to comment.