Skip to content

Commit b438921

Browse files
vladimirdolzhenkovladimirdolzhenko
committed
Fix DeleteRequest / GetRequest / UpdateRequest / ExplainRequest validation for null and/or empty id/type (elastic#35314)
Closes elastic#35297 (cherry picked from commit a467a81)
1 parent 566073e commit b438921

File tree

8 files changed

+159
-14
lines changed

8 files changed

+159
-14
lines changed

‎server/src/main/java/org/elasticsearch/action/delete/DeleteRequest.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.action.DocWriteRequest;
2525
import org.elasticsearch.action.support.replication.ReplicatedWriteRequest;
2626
import org.elasticsearch.common.Nullable;
27+
import org.elasticsearch.common.Strings;
2728
import org.elasticsearch.common.io.stream.StreamInput;
2829
import org.elasticsearch.common.io.stream.StreamOutput;
2930
import org.elasticsearch.common.lucene.uid.Versions;
@@ -84,13 +85,13 @@ public DeleteRequest(String index, String type, String id) {
8485
@Override
8586
public ActionRequestValidationException validate() {
8687
ActionRequestValidationException validationException = super.validate();
87-
if (type == null) {
88+
if (Strings.isEmpty(type)) {
8889
validationException = addValidationError("type is missing", validationException);
8990
}
90-
if (id == null) {
91+
if (Strings.isEmpty(id)) {
9192
validationException = addValidationError("id is missing", validationException);
9293
}
93-
if (!versionType.validateVersionForWrites(version)) {
94+
if (versionType.validateVersionForWrites(version) == false) {
9495
validationException = addValidationError("illegal version value [" + version + "] for version type ["
9596
+ versionType.name() + "]", validationException);
9697
}

‎server/src/main/java/org/elasticsearch/action/explain/ExplainRequest.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434

3535
import java.io.IOException;
3636

37+
import static org.elasticsearch.action.ValidateActions.addValidationError;
38+
3739
/**
3840
* Explain request encapsulating the explain query and document identifier to get an explanation for.
3941
*/
@@ -152,11 +154,11 @@ public ExplainRequest filteringAlias(AliasFilter filteringAlias) {
152154
@Override
153155
public ActionRequestValidationException validate() {
154156
ActionRequestValidationException validationException = super.validateNonNullIndex();
155-
if (type == null) {
156-
validationException = ValidateActions.addValidationError("type is missing", validationException);
157+
if (Strings.isEmpty(type)) {
158+
validationException = addValidationError("type is missing", validationException);
157159
}
158-
if (id == null) {
159-
validationException = ValidateActions.addValidationError("id is missing", validationException);
160+
if (Strings.isEmpty(id)) {
161+
validationException = addValidationError("id is missing", validationException);
160162
}
161163
if (query == null) {
162164
validationException = ValidateActions.addValidationError("query is missing", validationException);

‎server/src/main/java/org/elasticsearch/action/get/GetRequest.java

+8-5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.action.ValidateActions;
2525
import org.elasticsearch.action.support.single.shard.SingleShardRequest;
2626
import org.elasticsearch.common.Nullable;
27+
import org.elasticsearch.common.Strings;
2728
import org.elasticsearch.common.io.stream.StreamInput;
2829
import org.elasticsearch.common.io.stream.StreamOutput;
2930
import org.elasticsearch.common.lucene.uid.Versions;
@@ -32,6 +33,8 @@
3233

3334
import java.io.IOException;
3435

36+
import static org.elasticsearch.action.ValidateActions.addValidationError;
37+
3538
/**
3639
* A request to get a document (its source) from an index based on its type (optional) and id. Best created using
3740
* {@link org.elasticsearch.client.Requests#getRequest(String)}.
@@ -91,13 +94,13 @@ public GetRequest(String index, String type, String id) {
9194
@Override
9295
public ActionRequestValidationException validate() {
9396
ActionRequestValidationException validationException = super.validateNonNullIndex();
94-
if (type == null) {
95-
validationException = ValidateActions.addValidationError("type is missing", validationException);
97+
if (Strings.isEmpty(type)) {
98+
validationException = addValidationError("type is missing", validationException);
9699
}
97-
if (id == null) {
98-
validationException = ValidateActions.addValidationError("id is missing", validationException);
100+
if (Strings.isEmpty(id)) {
101+
validationException = addValidationError("id is missing", validationException);
99102
}
100-
if (!versionType.validateVersionForReads(version)) {
103+
if (versionType.validateVersionForReads(version) == false) {
101104
validationException = ValidateActions.addValidationError("illegal version value [" + version + "] for version type ["
102105
+ versionType.name() + "]", validationException);
103106
}

‎server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.action.support.replication.ReplicationRequest;
2929
import org.elasticsearch.action.support.single.instance.InstanceShardOperationRequest;
3030
import org.elasticsearch.common.Nullable;
31+
import org.elasticsearch.common.Strings;
3132
import org.elasticsearch.common.io.stream.StreamInput;
3233
import org.elasticsearch.common.io.stream.StreamOutput;
3334
import org.elasticsearch.common.logging.DeprecationLogger;
@@ -111,10 +112,10 @@ public ActionRequestValidationException validate() {
111112
if(upsertRequest != null && upsertRequest.version() != Versions.MATCH_ANY) {
112113
validationException = addValidationError("can't provide version in upsert request", validationException);
113114
}
114-
if (type == null) {
115+
if (Strings.isEmpty(type)) {
115116
validationException = addValidationError("type is missing", validationException);
116117
}
117-
if (id == null) {
118+
if (Strings.isEmpty(id)) {
118119
validationException = addValidationError("id is missing", validationException);
119120
}
120121

‎server/src/test/java/org/elasticsearch/action/delete/DeleteRequestTests.java

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.action.delete;
20+
21+
import org.elasticsearch.action.ActionRequestValidationException;
22+
import org.elasticsearch.test.ESTestCase;
23+
24+
import static org.hamcrest.CoreMatchers.hasItems;
25+
import static org.hamcrest.CoreMatchers.not;
26+
import static org.hamcrest.CoreMatchers.nullValue;
27+
28+
public class DeleteRequestTests extends ESTestCase {
29+
30+
public void testValidation() {
31+
{
32+
final DeleteRequest request = new DeleteRequest("index4", "_doc", "0");
33+
final ActionRequestValidationException validate = request.validate();
34+
35+
assertThat(validate, nullValue());
36+
}
37+
38+
{
39+
final DeleteRequest request = new DeleteRequest("index4", randomBoolean() ? "" : null, randomBoolean() ? "" : null);
40+
final ActionRequestValidationException validate = request.validate();
41+
42+
assertThat(validate, not(nullValue()));
43+
assertThat(validate.validationErrors(), hasItems("type is missing", "id is missing"));
44+
}
45+
}
46+
}

‎server/src/test/java/org/elasticsearch/action/explain/ExplainRequestTests.java

+25
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.elasticsearch.action.explain;
2020

21+
import org.elasticsearch.action.ActionRequestValidationException;
2122
import org.elasticsearch.common.io.stream.BytesStreamOutput;
2223
import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
2324
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
@@ -35,6 +36,10 @@
3536
import java.util.Collections;
3637
import java.util.List;
3738

39+
import static org.hamcrest.CoreMatchers.hasItems;
40+
import static org.hamcrest.CoreMatchers.not;
41+
import static org.hamcrest.CoreMatchers.nullValue;
42+
3843
public class ExplainRequestTests extends ESTestCase {
3944
private NamedWriteableRegistry namedWriteableRegistry;
4045

@@ -70,4 +75,24 @@ public void testSerialize() throws IOException {
7075
}
7176
}
7277
}
78+
79+
public void testValidation() {
80+
{
81+
final ExplainRequest request = new ExplainRequest("index4", "_doc", "0");
82+
request.query(QueryBuilders.termQuery("field", "value"));
83+
84+
final ActionRequestValidationException validate = request.validate();
85+
86+
assertThat(validate, nullValue());
87+
}
88+
89+
{
90+
final ExplainRequest request = new ExplainRequest("index4", randomBoolean() ? "" : null, randomBoolean() ? "" : null);
91+
request.query(QueryBuilders.termQuery("field", "value"));
92+
final ActionRequestValidationException validate = request.validate();
93+
94+
assertThat(validate, not(nullValue()));
95+
assertThat(validate.validationErrors(), hasItems("type is missing", "id is missing"));
96+
}
97+
}
7398
}

‎server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.action.get;
20+
21+
import org.elasticsearch.action.ActionRequestValidationException;
22+
import org.elasticsearch.test.ESTestCase;
23+
24+
import static org.hamcrest.CoreMatchers.hasItems;
25+
import static org.hamcrest.CoreMatchers.not;
26+
import static org.hamcrest.CoreMatchers.nullValue;
27+
28+
public class GetRequestTests extends ESTestCase {
29+
30+
public void testValidation() {
31+
{
32+
final GetRequest request = new GetRequest("index4", "_doc", "0");
33+
final ActionRequestValidationException validate = request.validate();
34+
35+
assertThat(validate, nullValue());
36+
}
37+
38+
{
39+
final GetRequest request = new GetRequest("index4", randomBoolean() ? "" : null, randomBoolean() ? "" : null);
40+
final ActionRequestValidationException validate = request.validate();
41+
42+
assertThat(validate, not(nullValue()));
43+
assertThat(validate.validationErrors(), hasItems("type is missing", "id is missing"));
44+
}
45+
}
46+
}

‎server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java

+21
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@
6161
import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
6262
import static org.elasticsearch.script.MockScriptEngine.mockInlineScript;
6363
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
64+
import static org.hamcrest.CoreMatchers.hasItems;
65+
import static org.hamcrest.CoreMatchers.nullValue;
6466
import static org.hamcrest.Matchers.arrayContaining;
6567
import static org.hamcrest.Matchers.contains;
6668
import static org.hamcrest.Matchers.empty;
@@ -564,6 +566,25 @@ public void testToValidateUpsertRequestWithVersion() {
564566
assertThat(updateRequest.validate().validationErrors(), contains("can't provide version in upsert request"));
565567
}
566568

569+
public void testValidate() {
570+
{
571+
UpdateRequest request = new UpdateRequest("index", "type", "id");
572+
request.doc("{}", XContentType.JSON);
573+
ActionRequestValidationException validate = request.validate();
574+
575+
assertThat(validate, nullValue());
576+
}
577+
578+
{
579+
UpdateRequest request = new UpdateRequest("index", randomBoolean() ? "" : null, randomBoolean() ? "" : null);
580+
request.doc("{}", XContentType.JSON);
581+
ActionRequestValidationException validate = request.validate();
582+
583+
assertThat(validate, not(nullValue()));
584+
assertThat(validate.validationErrors(), hasItems("type is missing", "id is missing"));
585+
}
586+
}
587+
567588
public void testParentAndRoutingExtraction() throws Exception {
568589
GetResult getResult = new GetResult("test", "type", "1", 0, false, null, null);
569590
IndexRequest indexRequest = new IndexRequest("test", "type", "1");

0 commit comments

Comments
 (0)