From 3befb8be941afb0f6e5bdf757ceda631291e5f60 Mon Sep 17 00:00:00 2001
From: Mikhail Khludnev <mkhl@apache.org>
Date: Thu, 27 Feb 2020 14:40:04 +0300
Subject: [PATCH] SOLR-13411: reject incremental update for route.field,
 uniqueKey and _version_.

---
 solr/CHANGES.txt                              |  2 ++
 .../processor/AtomicUpdateDocumentMerger.java |  9 +++++++--
 .../TestInPlaceUpdateWithRouteField.java      | 20 ++++++++++++-------
 .../org/apache/solr/update/TestUpdate.java    |  2 +-
 4 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 72a0a7c9c4b..a0f63b5e303 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -185,6 +185,8 @@ Bug Fixes
 
 * SOLR-14250: Do not log error when trying to consume non-existing input stream due to Expect: 100-continue (janhoy)
 
+* SOLR-13411: Deny atomic update for route.field, uniqueKey, version and throw exception. (Dr Oleg Savrasov via Mikhail Khludnev) 
+
 Other Changes
 ---------------------
 
diff --git a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
index 8bd51c244d3..9ebcde56210 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
@@ -183,12 +183,17 @@ public class AtomicUpdateDocumentMerger {
     // first pass, check the things that are virtually free,
     // and bail out early if anything is obviously not a valid in-place update
     for (String fieldName : sdoc.getFieldNames()) {
+      Object fieldValue = sdoc.getField(fieldName).getValue();
       if (fieldName.equals(uniqueKeyFieldName)
           || fieldName.equals(CommonParams.VERSION_FIELD)
           || fieldName.equals(routeFieldOrNull)) {
-        continue;
+        if (fieldValue instanceof Map ) {
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+              "Updating unique key, version or route field is not allowed: " + sdoc.getField(fieldName));
+        } else {
+          continue;
+        }
       }
-      Object fieldValue = sdoc.getField(fieldName).getValue();
       if (! (fieldValue instanceof Map) ) {
         // not an in-place update if there are fields that are not maps
         return Collections.emptySet();
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdateWithRouteField.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdateWithRouteField.java
index 14a48088d23..fc85fadfcd9 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdateWithRouteField.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdateWithRouteField.java
@@ -113,14 +113,20 @@ public class TestInPlaceUpdateWithRouteField extends SolrCloudTestCase {
         newVersion > initialVersion);
     Assert.assertThat( "Doc value must be updated", solrDocument.get("inplace_updatable_int"), is(newDocValue));
     Assert.assertThat("Lucene doc id should not be changed for In-Place Updates.", solrDocument.get("[docid]"), is(luceneDocId));
-    
+
+    sdoc.remove("shardName");
+    checkWrongCommandFailure(sdoc);
+
+    sdoc.addField("shardName",  map("set", "newShardName"));
+    checkWrongCommandFailure(sdoc);
+  }
+
+  private void checkWrongCommandFailure(SolrInputDocument sdoc) throws SolrServerException, IOException {
     try {
-      sdoc.remove("shardName");
-      new UpdateRequest()
-         .add(sdoc).process(cluster.getSolrClient(), COLLECTION);
-      fail("expect  an exception w/o route field");
-    }catch(SolrException ex) {
-      assertThat("expecting 400 in "+ex.getMessage(), ex.code(), is(400));
+      new UpdateRequest().add(sdoc).process(cluster.getSolrClient(), COLLECTION);
+      fail("expect an exception for wrong update command");
+    } catch (SolrException ex) {
+      assertThat("expecting 400 in " + ex.getMessage(), ex.code(), is(400));
     }
   }
 
diff --git a/solr/core/src/test/org/apache/solr/update/TestUpdate.java b/solr/core/src/test/org/apache/solr/update/TestUpdate.java
index f276dc75be5..24181697448 100644
--- a/solr/core/src/test/org/apache/solr/update/TestUpdate.java
+++ b/solr/core/src/test/org/apache/solr/update/TestUpdate.java
@@ -203,7 +203,7 @@ public class TestUpdate extends SolrTestCaseJ4 {
     );
     resetExceptionIgnores();
     assertEquals(400, se.code());
-    assertTrue(se.getMessage().contains("Invalid update of id field"));
+    assertTrue(se.getMessage().contains("Updating unique key, version or route field is not allowed"));
 
     afterUpdate.call();