Accept simple form encoded data for REST APIs
Simple cases like /review or /abandon can now accept standard form
values for basic properties, making it simple for tools to directly
post data:
curl -n --digest \
--data 'message=Does not compile.' \
--data labels.Verified=-1 \
http://localhost:8080/a/changes/3/revisions/1/review
Form field names are JSON field names in the top level object. If dot
appears in the name the part to the left is taken as the JSON field
name and the part to the right as the key for a Map. This nicely fits
with the labels structure used by /review, but doesn't support the
much more complex inline comment case. Clients that need to use more
complex fields must use JSON formatting for the request body.
Change-Id: Ica3db5db43c2fbc84f4cf7f36677e0cbbd116f26
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/ParameterParser.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/ParameterParser.java
index 51d98ff..1be1c03 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/ParameterParser.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/ParameterParser.java
@@ -18,12 +18,20 @@
import static com.google.gerrit.httpd.restapi.RestApiServlet.replyText;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Multimap;
+import com.google.common.collect.Sets;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.server.util.Url;
import com.google.gerrit.util.cli.CmdLineParser;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonPrimitive;
import com.google.inject.Inject;
import org.kohsuke.args4j.CmdLineException;
@@ -31,6 +39,8 @@
import java.io.IOException;
import java.io.StringWriter;
import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -91,4 +101,114 @@
}
}
}
+
+ private static Set<String> query(HttpServletRequest req) {
+ Set<String> params = Sets.newHashSet();
+ if (!Strings.isNullOrEmpty(req.getQueryString())) {
+ for (String kvPair : Splitter.on('&').split(req.getQueryString())) {
+ params.add(Iterables.getFirst(
+ Splitter.on('=').limit(2).split(kvPair),
+ null));
+ }
+ }
+ return params;
+ }
+
+ /**
+ * Convert a standard URL encoded form input into a parsed JSON tree.
+ * <p>
+ * Given an input such as:
+ *
+ * <pre>
+ * message=Does+not+compile.&labels.Verified=-1
+ * </pre>
+ *
+ * which is easily created using the curl command line tool:
+ *
+ * <pre>
+ * curl --data 'message=Does not compile.' --data labels.Verified=-1
+ * </pre>
+ *
+ * converts to a JSON object structure that is normally expected:
+ *
+ * <pre>
+ * {
+ * "message": "Does not compile.",
+ * "labels": {
+ * "Verified": "-1"
+ * }
+ * }
+ * </pre>
+ *
+ * This input can then be further processed into the Java input type expected
+ * by a view using Gson. Here we rely on Gson to perform implicit conversion
+ * of a string {@code "-1"} to a number type when the Java input type expects
+ * a number.
+ * <p>
+ * Conversion assumes any field name that does not contain {@code "."} will be
+ * a property of the top level input object. Any field with a dot will use the
+ * first segment as the top level property name naming an object, and the rest
+ * of the field name as a property in the nested object.
+ *
+ * @param req request to parse form input from and create JSON tree.
+ * @return the converted JSON object tree.
+ * @throws BadRequestException the request cannot be cast, as there are
+ * conflicting definitions for a nested object.
+ */
+ static JsonObject formToJson(HttpServletRequest req)
+ throws BadRequestException {
+ @SuppressWarnings("unchecked")
+ Map<String, String[]> map = req.getParameterMap();
+ return formToJson(map, query(req));
+ }
+
+ @VisibleForTesting
+ static JsonObject formToJson(Map<String, String[]> map, Set<String> query)
+ throws BadRequestException {
+ JsonObject inputObject = new JsonObject();
+ for (Map.Entry<String, String[]> ent : map.entrySet()) {
+ String key = ent.getKey();
+ String[] values = ent.getValue();
+
+ if (query.contains(key) || values.length == 0) {
+ // Disallow processing query parameters as input body fields.
+ // Implementations of views should avoid duplicate naming.
+ continue;
+ }
+
+ JsonObject obj = inputObject;
+ int dot = key.indexOf('.');
+ if (0 <= dot) {
+ String property = key.substring(0, dot);
+ JsonElement e = inputObject.get(property);
+ if (e == null) {
+ obj = new JsonObject();
+ inputObject.add(property, obj);
+ } else if (e.isJsonObject()) {
+ obj = e.getAsJsonObject();
+ } else {
+ throw new BadRequestException(String.format(
+ "key %s conflicts with %s",
+ key, property));
+ }
+ key = key.substring(dot + 1);
+ }
+
+ if (obj.get(key) != null) {
+ // This error should never happen. If all form values are handled
+ // together in a single pass properties are set only once. Setting
+ // again indicates something has gone very wrong.
+ throw new BadRequestException("invalid form input, use JSON instead");
+ } else if (values.length == 1) {
+ obj.addProperty(key, values[0]);
+ } else {
+ JsonArray list = new JsonArray();
+ for (String v : values) {
+ list.add(new JsonPrimitive(v));
+ }
+ obj.add(key, list);
+ }
+ }
+ return inputObject;
+ }
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 1f21bd5..d472b1d 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -108,6 +108,7 @@
/** MIME type used for a JSON response body. */
private static final String JSON_TYPE = JsonConstants.JSON_TYPE;
+ private static final String FORM_TYPE = "application/x-www-form-urlencoded";
private static final String UTF_8 = "UTF-8";
/**
@@ -303,6 +304,11 @@
} finally {
br.close();
}
+ } else if ("POST".equals(req.getMethod())
+ && isType(FORM_TYPE, req.getContentType())) {
+ return OutputFormat.JSON.newGson().fromJson(
+ ParameterParser.formToJson(req),
+ type);
} else {
throw new BadRequestException("Expected Content-Type: " + JSON_TYPE);
}
@@ -312,7 +318,7 @@
int len = req.getContentLength();
String type = req.getContentType();
return (len <= 0 && type == null)
- || (len == 0 && isType("application/x-www-form-urlencoded", type));
+ || (len == 0 && isType(FORM_TYPE, type));
}
private static boolean acceptsPutInput(Class<Object> type) {
diff --git a/gerrit-httpd/src/test/java/com/google/gerrit/httpd/restapi/ParameterParserTest.java b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/restapi/ParameterParserTest.java
new file mode 100644
index 0000000..ffbc7f3
--- /dev/null
+++ b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/restapi/ParameterParserTest.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2012 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd.restapi;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonPrimitive;
+
+import junit.framework.TestCase;
+
+public class ParameterParserTest extends TestCase {
+ public void testConvertFormToJson() throws BadRequestException {
+ JsonObject obj = ParameterParser.formToJson(
+ ImmutableMap.of(
+ "message", new String[]{"this.is.text"},
+ "labels.Verified", new String[]{"-1"},
+ "labels.Code-Review", new String[]{"2"},
+ "a_list", new String[]{"a", "b"}),
+ ImmutableSet.of("q"));
+
+ JsonObject labels = new JsonObject();
+ labels.addProperty("Verified", "-1");
+ labels.addProperty("Code-Review", "2");
+ JsonArray list = new JsonArray();
+ list.add(new JsonPrimitive("a"));
+ list.add(new JsonPrimitive("b"));
+ JsonObject exp = new JsonObject();
+ exp.addProperty("message", "this.is.text");
+ exp.add("labels", labels);
+ exp.add("a_list", list);
+
+ assertEquals(exp, obj);
+ }
+}
