bug id 3731 fix; do not allow resource methods to be annotated with @XXXParam

Change-Id: Ic71beb071954a1c05e7ed1412d5cbdb0253bf54e

Author: jansupol

core-server/src/main/java/org/glassfish/jersey/server/model/ResourceMethodValidator.java

@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 2012-2017 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012-2018 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -171,6 +171,23 @@ private void checkMethod(ResourceMethod method) {
             }
         }
 
+        // Prevent PARAM_ANNOTATION_SET annotations on a resource method
+        if (httpMethodAnnotations.size() != 0) {
+            checkUnexpectedAnnotations(method);
+        }
+    }
+
+    private void checkUnexpectedAnnotations(ResourceMethod resourceMethod) {
+        Invocable invocable = resourceMethod.getInvocable();
+        for (Annotation annotation : invocable.getHandlingMethod().getDeclaredAnnotations()) {
+            if (PARAM_ANNOTATION_SET.contains(annotation.annotationType())) {
+                Errors.fatal(resourceMethod, LocalizationMessages.METHOD_UNEXPECTED_ANNOTATION(
+                        invocable.getHandlingMethod().getName(),
+                        invocable.getHandler().getHandlerClass().getName(),
+                        annotation.annotationType().getName())
+                );
+            }
+        }
     }
 
     private void checkValueProviders(ResourceMethod method) {
@@ -191,6 +208,11 @@ private void visitSubResourceLocator(ResourceMethod locator) {
         if (void.class == invocable.getRawResponseType()) {
             Errors.fatal(locator, LocalizationMessages.SUBRES_LOC_RETURNS_VOID(invocable.getHandlingMethod()));
         }
+
+        // Prevent PARAM_ANNOTATION_SET annotations on a resource locator
+        if (invocable.getHandlingMethod().getAnnotation(Path.class) != null) {
+            checkUnexpectedAnnotations(locator);
+        }
     }
 
     private void checkParameters(ResourceMethod method) {

core-server/src/main/resources/org/glassfish/jersey/server/internal/localization.properties

@@ -1,7 +1,7 @@
 #
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
 #
-# Copyright (c) 2012-2017 Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2012-2018 Oracle and/or its affiliates. All rights reserved.
 #
 # The contents of this file are subject to the terms of either the GNU
 # General Public License Version 2 only ("GPL") or the Common Development
@@ -164,6 +164,7 @@ method.empty.path.annotation=The (sub)resource method {0} in {1} contains empty
 method.invocable.from.prematch.filters.only=Method can only be invoked from pre-matching request filters.
 method.parameter.cannot.be.null.or.empty=Method parameter "{0}" cannot be null or empty.
 method.parameter.cannot.be.null=Method parameter "{0}" cannot be null.
+method.unexpected.annotation=(Sub)resource method {0} in {1} contains unexpected annotation {2}.
 multiple.http.method.designators=A (sub-)resource method, {0}, should have only one HTTP method designator. It currently has the following designators defined: {1}
 new.ar.created.by.introspection.modeler=A new abstract resource created by IntrospectionModeler: {0}
 non.instantiable.component=Component of class {0} cannot be instantiated and will be ignored.

core-server/src/test/java/org/glassfish/jersey/server/model/ValidatorTest.java

@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 2010-2017 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010-2018 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -1150,6 +1150,62 @@ public void testVoidReturnType() throws Exception {
         assertEquals(Severity.HINT, issues.get(0).getSeverity());
     }
 
+    @Path("paramonsetter")
+    public static class ResourceWithParamOnSetter {
+        @QueryParam("id")
+        public void setId(String id) {
+        }
+
+        @GET
+        public String get() {
+            return "get";
+        }
+    }
+
+    @Test
+    public void testParamOnSetterIsOk() {
+        LOGGER.info("Validation should report no issues.");
+        List<ResourceModelIssue> issues = testResourceValidation(ResourceWithParamOnSetter.class);
+
+        assertEquals(0, issues.size());
+    }
+
+    @Path("paramonresourcepath")
+    public static class ResourceWithParamOnResourcePathAnnotatedMethod {
+        @QueryParam("id")
+        @Path("fail")
+        public String query() {
+            return "post";
+        }
+    }
+
+    @Test
+    public void testParamOnResourcePathAnnotatedMethodFails() {
+        LOGGER.info("Should report fatal during validation as @Path method should not be annotated with parameter annotation");
+        List<ResourceModelIssue> issues = testResourceValidation(ResourceWithParamOnResourcePathAnnotatedMethod.class);
+
+        assertEquals(1, issues.size());
+        assertEquals(Severity.FATAL, issues.get(0).getSeverity());
+    }
+
+    @Path("paramonresourceget")
+    public static class ResourceGETMethodFails {
+        @QueryParam("id")
+        @GET
+        public String get(@PathParam("abc") String id) {
+            return "get";
+        }
+    }
+
+    @Test
+    public void testParamOnResourceGETMethodFails() {
+        LOGGER.info("Should report fatal during validation as @GET method should not be annotated with parameter annotation");
+        List<ResourceModelIssue> issues = testResourceValidation(ResourceGETMethodFails.class);
+
+        assertEquals(1, issues.size());
+        assertEquals(Severity.FATAL, issues.get(0).getSeverity());
+    }
+
     /**
      * Test of disabled validation failing on errors.
      */
@@ -1180,11 +1236,13 @@ public void testDisableFailOnErrors() throws ExecutionException, InterruptedExce
                 PercentEncodedCaseSensitiveTest.class,
                 PercentEncodedTest.class,
                 ResourceAsProvider.class,
+                ResourceGETMethodFails.class,
                 ResourceMethodWithVoidReturnType.class,
                 ResourceRoot.class,
                 ResourceRootNotUnique.class,
                 ResourceSubPathRoot.class,
                 ResourceWithMultipleScopes.class,
+                ResourceWithParamOnResourcePathAnnotatedMethod.class,
                 TestAmbiguousParams.class,
                 TestAsyncGetRMReturningVoid.class,
                 TestEmptyPathSegment.class,