Skip to content
This repository was archived by the owner on Jun 30, 2023. It is now read-only.

check non-nullness of required parameters #148

Merged
tangiel merged 3 commits into from
Jun 11, 2018
Merged

check non-nullness of required parameters #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db1040c
check non-nullness of required parameters
tangiel Jun 8, 2018
18c13c4
undo else if refactor, which wasn't useful
tangiel Jun 11, 2018
aab3f07
fix indentation
tangiel Jun 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ...-framework/src/main/java/com/google/api/server/spi/request/ServletRequestParamReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.api.server.spi.auth.common.User;
import com.google.api.server.spi.config.AuthLevel;
import com.google.api.server.spi.config.Named;
import com.google.api.server.spi.config.Nullable;
import com.google.api.server.spi.config.annotationreader.AnnotationUtil;
import com.google.api.server.spi.config.model.ApiMethodConfig;
import com.google.api.server.spi.config.model.ApiSerializationConfig;
Expand Down Expand Up @@ -192,6 +193,9 @@ protected Object[] deserializeParams(JsonNode node) throws IOException, IllegalA
params[i] = objectReader.forType(clazz).readValue(nodeValue);
}
}
if (params[i] == null && isRequiredParameter(method, i)) {
throw new BadRequestException("null value for parameter '" + name + "' not allowed");
}
logger.log(Level.FINE, "deserialize: {0} {1} injected into param[{2}] named {3}",
new Object[] {clazz, params[i], i, name});
}
Expand All @@ -200,6 +204,11 @@ protected Object[] deserializeParams(JsonNode node) throws IOException, IllegalA
return params;
}

private boolean isRequiredParameter(EndpointMethod method, int i) {
return AnnotationUtil.getNullableParameter(method.getMethod(), i, Nullable.class) == null
|| method.getParameterTypes()[i].isPrimitive();
}

@VisibleForTesting
User getUser() throws ServiceException {
return Auth.from(endpointsContext.getRequest()).authenticate();
Expand Down
5 changes: 4 additions & 1 deletion ...rk/src/test/java/com/google/api/server/spi/request/RestServletRequestParamReaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.api.server.spi.config.ApiMethod.HttpMethod;
import com.google.api.server.spi.config.DefaultValue;
import com.google.api.server.spi.config.Named;
import com.google.api.server.spi.config.Nullable;
import com.google.api.server.spi.config.annotationreader.ApiConfigAnnotationReader;
import com.google.api.server.spi.config.model.ApiConfig;
import com.google.api.server.spi.config.model.ApiMethodConfig;
Expand Down Expand Up @@ -229,7 +230,9 @@ public boolean equals(Object o) {
@Api
public static class TestApi {
@ApiMethod(name = "test", httpMethod = HttpMethod.GET, path = "test/{path}")
public void test(@Named("path") long path, @Named("dates") List<SimpleDate> dates,
public void test(
@Nullable @Named("path") long path,
@Nullable @Named("dates") List<SimpleDate> dates,
@Named("defaultvalue") @DefaultValue("2015-01-01") SimpleDate defaultValue,
TestResource resource) {
}
Expand Down
57 changes: 39 additions & 18 deletions ...mework/src/test/java/com/google/api/server/spi/request/ServletRequestParamReaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.api.server.spi.config.Named;
import com.google.api.server.spi.config.Nullable;
import com.google.api.server.spi.config.model.ApiMethodConfig;
import com.google.api.server.spi.response.BadRequestException;
import com.google.api.server.spi.response.UnauthorizedException;
import com.google.api.server.spi.testing.TestEndpoint;
import com.google.api.server.spi.testing.TestEndpoint.Request;
Expand All @@ -39,14 +40,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.reflect.TypeToken;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.runners.MockitoJUnitRunner;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -61,10 +54,15 @@
import java.util.Map;
import java.util.Set;
import java.util.TimeZone;

import javax.servlet.ServletContext;
import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.runners.MockitoJUnitRunner;

/**
* Tests for {@link ServletRequestParamReader}.
Expand Down Expand Up @@ -150,25 +148,28 @@ public void testReadMissingParameters() throws Exception {
.put(TestEndpoint.NAME_STRING, "\"" + VALUE_STRING + "\"")
.put(TestEndpoint.NAME_BOOLEAN, String.valueOf(VALUE_BOOLEAN))
.put(TestEndpoint.NAME_LONG, String.valueOf(VALUE_LONG))
.put(TestEndpoint.NAME_LONG_OBJECT, String.valueOf(VALUE_LONG))
.put(TestEndpoint.NAME_DOUBLE, String.valueOf(VALUE_DOUBLE))
.put(TestEndpoint.NAME_DOUBLE_OBJECT, String.valueOf(VALUE_DOUBLE))
.put(TestEndpoint.NAME_BOOLEAN_OBJECT, String.valueOf(VALUE_BOOLEAN))
.put(TestEndpoint.NAME_INTEGER, String.valueOf(VALUE_INTEGER))
.put(TestEndpoint.NAME_INTEGER_OBJECT, String.valueOf(VALUE_INTEGER))
.put(TestEndpoint.NAME_LONG_OBJECT, String.valueOf(VALUE_LONG))
.put(TestEndpoint.NAME_FLOAT, String.valueOf(VALUE_FLOAT))
.put(TestEndpoint.NAME_FLOAT_OBJECT, String.valueOf(VALUE_FLOAT))
.put("stringValue", "321")
.put("more", "999").build());

assertEquals(VALUE_STRING, params[0]);
assertEquals(VALUE_BOOLEAN, params[1]);
assertNull(params[2]);
assertEquals(VALUE_INTEGER, params[2]);
assertEquals(VALUE_LONG, params[3]);
assertNull(params[4]);
assertEquals(VALUE_FLOAT, params[4]);
assertEquals(VALUE_DOUBLE, params[5]);
assertEquals(VALUE_BOOLEAN, params[6]);
assertEquals(VALUE_INTEGER, params[7]);
assertEquals(VALUE_LONG, params[8]);
assertEquals(VALUE_FLOAT, params[9]);
assertNull(params[10]);
assertEquals(VALUE_DOUBLE, params[10]);
assertEquals("321", ((Request) params[11]).getStringValue());
assertEquals(-1, (int) ((Request) params[11]).getIntegerValue());
assertEquals(USER, params[12]);
Expand Down Expand Up @@ -586,11 +587,13 @@ public void foo(@Named("str") String string,
public void testJavaxNamed() throws Exception {
class Test {
@SuppressWarnings("unused")
public void foo(@javax.inject.Named("str") String str, @javax.inject.Named("i") int i) {}
public void foo(
@javax.inject.Named("str") String str,
@Nullable @javax.inject.Named("i") Integer i) {}
}
String requestString = "{\"str\":\"hello\"}";

Method method = Test.class.getDeclaredMethod("foo", String.class, int.class);
Method method = Test.class.getDeclaredMethod("foo", String.class, Integer.class);
Object[] params = readParameters(requestString, method);

assertEquals(2, params.length);
Expand All @@ -602,7 +605,7 @@ public void foo(@javax.inject.Named("str") String str, @javax.inject.Named("i")
public void testCachedNamesAreUsed() throws Exception {
class Test {
@SuppressWarnings("unused")
public void foo(@Named("foo1") String f1, @Named("foo2") String f2,
public void foo(@Named("foo1") String f1, @Nullable @Named("foo2") String f2,
@Named("foo3") String f3) {}
}

Expand All @@ -624,8 +627,10 @@ public void foo(@Named("foo1") String f1, @Named("foo2") String f2,
public void testNamesAreCached() throws Exception {
class Test {
@SuppressWarnings("unused")
public void foo(@Named("foo1") String f1, @Named("foo2") String f2,
@Named("foo3") String f3) {}
public void foo(
@Nullable @Named("foo1") String f1,
@Nullable @Named("foo2") String f2,
@Nullable @Named("foo3") String f3) {}
}

Method method = Test.class.getDeclaredMethod("foo", String.class, String.class, String.class);
Expand Down Expand Up @@ -832,6 +837,22 @@ public void getUser(com.google.appengine.api.users.User user) { }
}
}

@Test
public void testNullValueForRequiredParam() throws Exception {
class TestNullValueForRequiredParam {
@SuppressWarnings("unused")
public void test(@Named("testParam") String testParam) {}
}
try {
Object[] params =
readParameters("{}",
TestNullValueForRequiredParam.class.getDeclaredMethod("test", String.class));
fail("expected bad request exception");
} catch (BadRequestException ex) {
// expected
}
}

private Object[] readParameters(String input, Method method) throws Exception {
return readParameters(input, EndpointMethod.create(method.getDeclaringClass(), method));
}
Expand Down