Apply a workaround for servlet path and path info (#369)

Apply a workaround for servlet path and path info

If a servlet is mapped to the root then forwarding is used.
In this case the servlet path becomes the path info and path info is
null. To be able to make Vaadin servlet logic work in a proper way the
HttpServletRequest instance is replaced with a wrapper which returns
expected values for the servlet path and path info.

Fixes #331

vaadin-spring/src/main/java/com/vaadin/flow/spring/ForwardingRequestWrapper.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2000-2018 Vaadin Ltd.
+ *
+ * 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.vaadin.flow.spring;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+
+import org.springframework.web.util.UrlPathHelper;
+
+import com.vaadin.flow.server.VaadinServlet;
+
+/**
+ * Creates an {@link HttpServletRequestWrapper} instance which returns
+ * appropriate {@code servletPath} and {@code pathInfo} (which are expected by
+ * {@link VaadinServlet}) in case forwarding is enforced.
+ *
+ * @see <a href="https://jira.spring.io/browse/SPR-17457">Spring issue</a>
+ *
+ * @author Vaadin Ltd
+ *
+ */
+public class ForwardingRequestWrapper extends HttpServletRequestWrapper {
+
+    private UrlPathHelper urlPathHelper = new UrlPathHelper();
+
+    public ForwardingRequestWrapper(HttpServletRequest request) {
+        super(request);
+    }
+
+    @Override
+    public String getServletPath() {
+        String pathInfo = super.getPathInfo();
+        if (pathInfo == null) {
+            // the path where a ServletForwardingController is registered is not
+            // a real servlet path
+            return "";
+        } else {
+            return super.getServletPath();
+        }
+    }
+
+    @Override
+    public String getPathInfo() {
+        String pathInfo = super.getPathInfo();
+        if (pathInfo == null) {
+            // this uses getServletPath() and should work both with and without
+            // clearServletPath
+            pathInfo = urlPathHelper.getPathWithinServletMapping(this);
+        }
+        return pathInfo;
+    }
+}

vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringBootAutoConfiguration.java

@@ -21,19 +21,15 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.AutoConfigureBefore;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
-import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
-import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletRegistrationBean;
 import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.boot.web.servlet.ServletContextInitializer;
 import org.springframework.boot.web.servlet.ServletRegistrationBean;
 import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Conditional;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
 import org.springframework.util.ClassUtils;
 import org.springframework.web.context.WebApplicationContext;
-import org.springframework.web.servlet.DispatcherServlet;
 import org.springframework.web.socket.server.standard.ServerEndpointExporter;
 
 import com.vaadin.flow.server.Constants;
@@ -48,8 +44,7 @@
 @AutoConfigureBefore(WebMvcAutoConfiguration.class)
 @ConditionalOnClass(ServletContextInitializer.class)
 @EnableConfigurationProperties(VaadinConfigurationProperties.class)
-@Import({ VaadinServletConfiguration.class,
-        DispatcherServletRegistrationBeanConfig.class })
+@Import({ VaadinServletConfiguration.class })
 public class SpringBootAutoConfiguration {
 
     @Autowired
@@ -78,14 +73,15 @@ public ServletContextInitializer contextInitializer() {
     public ServletRegistrationBean<SpringServlet> servletRegistrationBean() {
         String mapping = configurationProperties.getUrlMapping();
         Map<String, String> initParameters = new HashMap<>();
-        if (RootMappedCondition.isRootMapping(mapping)) {
+        boolean rootMapping = RootMappedCondition.isRootMapping(mapping);
+        if (rootMapping) {
             mapping = VaadinServletConfiguration.VAADIN_SERVLET_MAPPING;
             initParameters.put(Constants.SERVLET_PARAMETER_PUSH_URL,
                     VaadinMVCWebAppInitializer
                             .makeContextRelative(mapping.replace("*", "")));
         }
         ServletRegistrationBean<SpringServlet> registration = new ServletRegistrationBean<>(
-                new SpringServlet(context), mapping);
+                new SpringServlet(context, rootMapping), mapping);
         registration.setInitParameters(initParameters);
         registration
                 .setAsyncSupported(configurationProperties.isAsyncSupported());
@@ -94,34 +90,6 @@ public ServletRegistrationBean<SpringServlet> servletRegistrationBean() {
         return registration;
     }
 
-    /**
-     * Creates a {@link DispatcherServletRegistrationBean} instance for a
-     * dispatcher servlet in case Vaadin servlet is mapped to the root.
-     * <p>
-     * This is needed for correct servlet path (and path info) values available
-     * in Vaadin servlet because it works via forwarding controller which is not
-     * properly mapped without this registration.
-     * <p>
-     * In the modern versions of Spring Boot this is done via extra
-     * {@link DispatcherServletRegistrationBeanConfig} configuration because
-     * {@link DispatcherServletRegistrationBean} bean should be used instead of
-     * {@code ServletRegistrationBean<DispatcherServlet>}. So this method works
-     * only if there is no {@link DispatcherServletRegistrationBean} class.
-     *
-     * @return a custom DispatcherServletRegistrationBean instance for
-     *         dispatcher servlet
-     */
-    @Bean
-    @Conditional(RootMappedCondition.class)
-    @ConditionalOnMissingClass("org.springframework.boot.autoconfigure.web.servlet.DispatcherServletRegistrationBean")
-    public ServletRegistrationBean<DispatcherServlet> dispatcherServletRegistration() {
-        DispatcherServlet servlet = context.getBean(DispatcherServlet.class);
-        ServletRegistrationBean<DispatcherServlet> registration = new ServletRegistrationBean<>(
-                servlet, "/*");
-        registration.setName("dispatcher");
-        return registration;
-    }
-
     /**
      * Deploys JSR-356 websocket endpoints when Atmosphere is available.
      *

vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringServlet.java

@@ -15,8 +15,13 @@
  */
 package com.vaadin.flow.spring;
 
+import java.io.IOException;
 import java.util.Properties;
 
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
 import org.springframework.context.ApplicationContext;
 import org.springframework.core.env.Environment;
 
@@ -35,6 +40,7 @@
 public class SpringServlet extends VaadinServlet {
 
     private final ApplicationContext context;
+    private final boolean forwardingEnforced;
 
     /**
      * Creates a new Vaadin servlet instance with the application
@@ -43,8 +49,16 @@ public class SpringServlet extends VaadinServlet {
      * @param context
      *            the Spring application context
      */
-    public SpringServlet(ApplicationContext context) {
+    public SpringServlet(ApplicationContext context,
+            boolean forwardingEnforced) {
         this.context = context;
+        this.forwardingEnforced = forwardingEnforced;
+    }
+
+    @Override
+    protected void service(HttpServletRequest request,
+            HttpServletResponse response) throws ServletException, IOException {
+        super.service(wrapRequest(request), response);
     }
 
     @Override
@@ -65,6 +79,18 @@ protected DeploymentConfiguration createDeploymentConfiguration(
         return super.createDeploymentConfiguration(properties);
     }
 
+    private HttpServletRequest wrapRequest(HttpServletRequest request) {
+        if (forwardingEnforced && request.getPathInfo() == null) {
+            /*
+             * We need to apply a workaround in case of forwarding
+             *
+             * see https://jira.spring.io/browse/SPR-17457
+             */
+            return new ForwardingRequestWrapper(request);
+        }
+        return request;
+    }
+
     private void config(Properties properties) {
         setProperty(Constants.SERVLET_PARAMETER_PRODUCTION_MODE, properties);
         setProperty(Constants.SERVLET_PARAMETER_DISABLE_XSRF_PROTECTION,

vaadin-spring/src/main/java/com/vaadin/flow/spring/VaadinMVCWebAppInitializer.java

@@ -54,15 +54,19 @@ public void onStartup(ServletContext servletContext)
         servletContext.addListener(new ContextLoaderListener(context));
 
         context.refresh();
+
         Environment env = context.getBean(Environment.class);
+        String mapping = env
+                .getProperty(RootMappedCondition.URL_MAPPING_PROPERTY, "/*");
+
+        boolean rootMapping = RootMappedCondition.isRootMapping(mapping);
+
         Dynamic registration = servletContext.addServlet(
                 ClassUtils.getShortNameAsProperty(SpringServlet.class),
-                new SpringServlet(context));
+                new SpringServlet(context, rootMapping));
 
-        String mapping = env
-                .getProperty(RootMappedCondition.URL_MAPPING_PROPERTY, "/*");
         Map<String, String> initParameters = new HashMap<>();
-        if (RootMappedCondition.isRootMapping(mapping)) {
+        if (rootMapping) {
             Dynamic dispatcherRegistration = servletContext
                     .addServlet("dispatcher", new DispatcherServlet(context));
             dispatcherRegistration.addMapping("/*");

vaadin-spring/src/test/java/com/vaadin/flow/spring/SpringClassesSerializableTest.java

@@ -58,6 +58,7 @@ public void run() {
     @Override
     protected Stream<String> getExcludedPatterns() {
         return Stream.concat(Stream.of(
+                "com\\.vaadin\\.flow\\.spring\\.ForwardingRequestWrapper",
                 "com\\.vaadin\\.flow\\.spring\\.VaadinScanPackagesRegistrar",
                 "com\\.vaadin\\.flow\\.spring\\.VaadinScanPackagesRegistrar\\$VaadinScanPackages",
                 "com\\.vaadin\\.flow\\.spring\\.VaadinServletContextInitializer",

vaadin-spring/src/test/java/com/vaadin/flow/spring/instantiator/SpringInstantiatorTest.java

@@ -143,7 +143,7 @@ public void getI18NProvider_i18nProviderIsABean_i18nProviderIsAvailable()
 
     public static VaadinServletService getService(ApplicationContext context,
             Properties configProperties) throws ServletException {
-        SpringServlet servlet = new SpringServlet(context) {
+        SpringServlet servlet = new SpringServlet(context, false) {
             @Override
             protected DeploymentConfiguration createDeploymentConfiguration(
                     Properties initParameters) {