This is an automated email from the ASF dual-hosted git repository.
prabhjyotsingh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new a792b50 ZEPPELIN-4679: Add API validation for creating interpreter
a792b50 is described below
commit a792b5028ccdfce9bb29ad876fdde11cbbb62f98
Author: Prabhjyot Singh <prabhjyotsi...@gmail.com>
AuthorDate: Thu Mar 12 15:19:23 2020 +0530
ZEPPELIN-4679: Add API validation for creating interpreter
### What is this PR for?
This is an extension of ZEPPELIN-4377, I believe we should have an API
validation for the same.
### What type of PR is it?
[Bug Fix]
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-4679
### How should this be tested?
Calling directly API should fail, and if somehow this is by pass from UI,
UI should also return proper error.
```
curl 'http://localhost:8080/api/interpreter/setting' -H 'Content-Type:
application/json;charset=UTF-8' --data-binary '{"name":"test
space","group":"angular","properties":{},"dependencies":[],"option":{"remote":true,"isExistingProcess":false,"setPermission":false,"session":false,"process":false,"perNote":"shared","perUser":"shared","owners":[]},"propertyValue":"","propertyKey":"","propertyType":"textarea"}'
```
### Screenshots (if appropriate)
N/A
### Questions:
* Does the licenses files need update? N/A
* Is there breaking changes for older versions? N/A
* Does this needs documentation? N/A
Author: Prabhjyot Singh <prabhjyotsi...@gmail.com>
Closes #3684 from prabhjyotsingh/ZEPPELIN-4679 and squashes the following
commits:
1fdddaf [Prabhjyot Singh] fix travis
485f8ff [Prabhjyot Singh] make pattern static constant to avoid its
compilation everytime
d787e75 [Prabhjyot Singh] add unit test
2c7a406 [Prabhjyot Singh] ZEPPELIN-4679: add API validation
---
.../zeppelin/rest/InterpreterRestApiTest.java | 57 ++++++++++++++++++++++
.../interpreter/InterpreterSettingManager.java | 19 +++++---
2 files changed, 70 insertions(+), 6 deletions(-)
diff --git
a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
index 9819b87..48d6c51 100644
---
a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
+++
b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
@@ -25,6 +25,7 @@ import org.apache.commons.httpclient.methods.DeleteMethod;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.commons.httpclient.methods.PostMethod;
import org.apache.commons.httpclient.methods.PutMethod;
+import org.apache.commons.lang3.StringUtils;
import org.apache.zeppelin.interpreter.InterpreterOption;
import org.apache.zeppelin.interpreter.InterpreterSetting;
import org.apache.zeppelin.notebook.Note;
@@ -234,6 +235,62 @@ public class InterpreterRestApiTest extends
AbstractTestRestApi {
post.releaseConnection();
}
+ @Test
+ public void testSettingsCreateWithInvalidName() throws IOException {
+ String reqBody = "{"
+ + "\"name\": \"mdName\","
+ + "\"group\": \"md\","
+ + "\"properties\": {"
+ + "\"propname\": {"
+ + "\"value\": \"propvalue\","
+ + "\"name\": \"propname\","
+ + "\"type\": \"textarea\""
+ + "}"
+ + "},"
+ + "\"interpreterGroup\": ["
+ + "{"
+ + "\"class\": \"org.apache.zeppelin.markdown.Markdown\","
+ + "\"name\": \"md\""
+ + "}"
+ + "],"
+ + "\"dependencies\": [],"
+ + "\"option\": {"
+ + "\"remote\": true,"
+ + "\"session\": false"
+ + "}"
+ + "}";
+ JsonObject jsonRequest = gson.fromJson(StringUtils.replace(reqBody,
"mdName", "mdValidName"), JsonElement.class).getAsJsonObject();
+ PostMethod post = httpPost("/interpreter/setting/",
jsonRequest.toString());
+ String postResponse = post.getResponseBodyAsString();
+ LOG.info("testSetting with valid name\n" + post.getResponseBodyAsString());
+ InterpreterSetting created =
convertResponseToInterpreterSetting(postResponse);
+ String newSettingId = created.getId();
+ // then : call create setting API
+ assertThat("test create method:", post, isAllowed());
+ post.releaseConnection();
+
+ // when: call delete setting API
+ DeleteMethod delete = httpDelete("/interpreter/setting/" + newSettingId);
+ LOG.info("testSetting delete response\n" +
delete.getResponseBodyAsString());
+ // then: call delete setting API
+ assertThat("Test delete method:", delete, isAllowed());
+ delete.releaseConnection();
+
+
+ JsonObject jsonRequest2 = gson.fromJson(StringUtils.replace(reqBody,
"mdName", "name space"), JsonElement.class).getAsJsonObject();
+ PostMethod post2 = httpPost("/interpreter/setting/",
jsonRequest2.toString());
+ LOG.info("testSetting with name with space\n" +
post2.getResponseBodyAsString());
+ assertThat("test create method with space:", post2, isNotFound());
+ post2.releaseConnection();
+
+ JsonObject jsonRequest3 = gson.fromJson(StringUtils.replace(reqBody,
"mdName", ""), JsonElement.class).getAsJsonObject();
+ PostMethod post3 = httpPost("/interpreter/setting/",
jsonRequest3.toString());
+ LOG.info("testSetting with empty name\n" +
post3.getResponseBodyAsString());
+ assertThat("test create method with empty name:", post3, isNotFound());
+ post3.releaseConnection();
+
+ }
+
public void testInterpreterRestart() throws IOException,
InterruptedException {
Note note = null;
try {
diff --git
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
index 415f1a1..69bdf9d 100644
---
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
+++
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
@@ -26,15 +26,14 @@ import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.reflect.TypeToken;
-import java.util.Properties;
import java.util.LinkedHashMap;
import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import javax.inject.Inject;
import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.ArrayUtils;
import org.apache.commons.lang.StringUtils;
-import org.apache.hadoop.yarn.webapp.hamlet.Hamlet;
import org.apache.zeppelin.cluster.ClusterManagerServer;
import org.apache.zeppelin.cluster.event.ClusterEvent;
import org.apache.zeppelin.cluster.event.ClusterEventListener;
@@ -105,6 +104,7 @@ import static
org.apache.zeppelin.cluster.ClusterManagerServer.CLUSTER_INTP_SETT
@ManagedObject("interpreterSettingManager")
public class InterpreterSettingManager implements NoteEventListener,
ClusterEventListener {
+ private static final Pattern VALID_INTERPRETER_NAME =
Pattern.compile("^[-_a-zA-Z0-9]+$");
private static final Logger LOGGER =
LoggerFactory.getLogger(InterpreterSettingManager.class);
private static final Map<String, Object> DEFAULT_EDITOR = ImmutableMap.of(
"language", (Object) "text",
@@ -767,9 +767,16 @@ public class InterpreterSettingManager implements
NoteEventListener, ClusterEven
InterpreterOption option,
Map<String,
InterpreterProperty> properties)
throws IOException {
- if (name.indexOf(".") >= 0) {
- throw new IOException("'.' is invalid for InterpreterSetting name.");
+ if (name == null) {
+ throw new IOException("Interpreter name shouldn't be empty.");
}
+
+ // check if name is valid
+ Matcher matcher = VALID_INTERPRETER_NAME.matcher(name);
+ if(!matcher.find()){
+ throw new IOException("Interpreter name shouldn't be empty, and can
consist only of: -_a-zA-Z0-9");
+ }
+
// check if name is existed
for (InterpreterSetting interpreterSetting : interpreterSettings.values())
{
if (interpreterSetting.getName().equals(name)) {
@@ -827,7 +834,7 @@ public class InterpreterSettingManager implements
NoteEventListener, ClusterEven
dependencyResolver.delRepo(id);
saveToFile();
}
-
+
/** restart in interpreter setting page */
private InterpreterSetting inlineSetPropertyAndRestart(
String id,
