spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.86k stars 40.61k forks source link

Error message getting replaced if undertow is used #15665

Closed pperez closed 5 years ago

pperez commented 5 years ago

Spring Boot version: 2.1.1.RELEASE I'm integrating Spring Cloud Config Server with vault, if i use undertow as my embedded server, and try to get data from vault without the X-Config-Token, an IllegalArgumentException is thrown (org.springframework.cloud.config.server.environment.VaultEnvironmentRepository with this message ""Missing required header: X-Config-Token"; when using tomcat, it returns the following response:

{"timestamp":"2019-01-09T14:53:50.996+0000","status":400,"error":"Bad Request","message":"Missing required header: X-Config-Token","path":"/poc-vault/dev"}

If i use undertow:

{"timestamp":"2019-01-09T14:58:03.927+0000","status":400,"error":"Bad Request","message":"Bad Request","path":"/poc-vault/dev"}

On org.springframework.boot.web.servlet.error.DefaultErrorAttributes addErrorDetails method, it checks if the attribute javax.servlet.error.message is not empty, if it is, it replaces the message with that, on undertow it says "Bad Request" but on tomcat it's just "". I think the attribute is getting filled elsewhere, but i don't know why this happens on undertow but not on tomcat

pperez commented 5 years ago

PS: It also happens on 2.0.7.RELEASE

wilkinsona commented 5 years ago

I think there must be more to it than different behaviour in Tomcat and Undertow.

In a minimal example with a @RestController that throws an IllegalArgumentException I get a 500 response (which is what I would expect) rather than the 400 that you're seeing. Also, the message with Tomcat is Missing required header: X-Config-Token and with Undertow it's Request processing failed; nested exception is java.lang.IllegalArgumentException: Missing required header: X-Config-Token.

Can you please provide a minimal example that reproduces the behaviour that you've described. Something that doesn't require Spring Cloud would be ideal as that would help to eliminate a number of possible causes that are out of Boot's control.

pperez commented 5 years ago

I @wilkinsona i've generated a minimal example here https://github.com/pperez/sb-15665 It has two branches, tomcat and undertow. There was a exceptionhandler doing that transformation, i've copied that from the cloud implementation

wilkinsona commented 5 years ago

Tomcat sets the message to "" here:

Daemon Thread [http-nio-8080-exec-1] (Suspended (entry into method setAttribute in Request))    
    owns: NioEndpoint$NioSocketWrapper  (id=84) 
    Request.setAttribute(String, Object) line: 1490 
    StandardHostValve.status(Request, Response) line: 239   
    StandardHostValve.invoke(Request, Response) line: 175   
    ErrorReportValve.invoke(Request, Response) line: 92 
    StandardEngineValve.invoke(Request, Response) line: 74  
    CoyoteAdapter.service(Request, Response) line: 343  
    Http11Processor.service(SocketWrapperBase<?>) line: 408 
    Http11Processor(AbstractProcessorLight).process(SocketWrapperBase<?>, SocketEvent) line: 66 
    AbstractProtocol$ConnectionHandler<S>.process(SocketWrapperBase<S>, SocketEvent) line: 834  
    NioEndpoint$SocketProcessor.doRun() line: 1417  
    NioEndpoint$SocketProcessor(SocketProcessorBase<S>).run() line: 49  
    ThreadPoolExecutor(ThreadPoolExecutor).runWorker(ThreadPoolExecutor$Worker) line: 1149  
    ThreadPoolExecutor$Worker.run() line: 624   
    TaskThread$WrappingRunnable.run() line: 61  
    TaskThread(Thread).run() line: 748  

Undertow sets the message to Bad Request here:

Thread [XNIO-1 task-1] (Suspended (entry into method setAttribute in HttpServletRequestImpl))   
    HttpServletRequestImpl.setAttribute(String, Object) line: 917   
    RequestDispatcherImpl.error(ServletRequestContext, ServletRequest, ServletResponse, String, Throwable, String) line: 455    
    RequestDispatcherImpl.error(ServletRequestContext, ServletRequest, ServletResponse, String, String) line: 408   
    HttpServletResponseImpl.doErrorDispatch(int, String) line: 164  
    ServletInitialHandler.handleFirstRequest(HttpServerExchange, ServletRequestContext) line: 299   
    ServletInitialHandler.access$100(ServletInitialHandler, HttpServerExchange, ServletRequestContext) line: 81 
    ServletInitialHandler$2.call(HttpServerExchange, ServletRequestContext) line: 138   
    ServletInitialHandler$2.call(HttpServerExchange, Object) line: 135  
    ServletRequestContextThreadSetupAction$1.call(HttpServerExchange, C) line: 48   
    ContextClassLoaderSetupAction$1.call(HttpServerExchange, C) line: 43    
    ServletInitialHandler.dispatchRequest(HttpServerExchange, ServletRequestContext, ServletChain, DispatcherType) line: 272    
    ServletInitialHandler.access$000(ServletInitialHandler, HttpServerExchange, ServletRequestContext, ServletChain, DispatcherType) line: 81   
    ServletInitialHandler$1.handleRequest(HttpServerExchange) line: 104 
    Connectors.executeRootHandler(HttpHandler, HttpServerExchange) line: 360    
    HttpServerExchange$1.run() line: 830    
    XnioWorker$TaskPool(ThreadPoolExecutor).runWorker(ThreadPoolExecutor$Worker) line: 1149 
    ThreadPoolExecutor$Worker.run() line: 624   
    Thread.run() line: 748  

The message is Bad request due to the call to sendError(int):

Thread [XNIO-1 task-1] (Suspended (entry into method setError in ServletRequestContext))    
    ServletRequestContext.setError(int, String) line: 251   
    HttpServletResponseImpl.sendError(int, String) line: 147    
    HttpServletResponseImpl.sendError(int) line: 183    
    VaultController.illegalArgument(HttpServletResponse) line: 26   
    NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method]  
    NativeMethodAccessorImpl.invoke(Object, Object[]) line: 62  
    DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43  
    Method.invoke(Object, Object...) line: 498  
    ServletInvocableHandlerMethod(InvocableHandlerMethod).doInvoke(Object...) line: 189 
    ServletInvocableHandlerMethod(InvocableHandlerMethod).invokeForRequest(NativeWebRequest, ModelAndViewContainer, Object...) line: 138    
    ServletInvocableHandlerMethod.invokeAndHandle(ServletWebRequest, ModelAndViewContainer, Object...) line: 102    
    ExceptionHandlerExceptionResolver.doResolveHandlerMethodException(HttpServletRequest, HttpServletResponse, HandlerMethod, Exception) line: 412  
    ExceptionHandlerExceptionResolver(AbstractHandlerMethodExceptionResolver).doResolveException(HttpServletRequest, HttpServletResponse, Object, Exception) line: 61   
    ExceptionHandlerExceptionResolver(AbstractHandlerExceptionResolver).resolveException(HttpServletRequest, HttpServletResponse, Object, Exception) line: 136  
    HandlerExceptionResolverComposite.resolveException(HttpServletRequest, HttpServletResponse, Object, Exception) line: 80 
    DispatcherServlet.processHandlerException(HttpServletRequest, HttpServletResponse, Object, Exception) line: 1297    
    DispatcherServlet.processDispatchResult(HttpServletRequest, HttpServletResponse, HandlerExecutionChain, ModelAndView, Exception) line: 1109 
    DispatcherServlet.doDispatch(HttpServletRequest, HttpServletResponse) line: 1055    
    DispatcherServlet.doService(HttpServletRequest, HttpServletResponse) line: 942  
    DispatcherServlet(FrameworkServlet).processRequest(HttpServletRequest, HttpServletResponse) line: 1005  
    DispatcherServlet(FrameworkServlet).doGet(HttpServletRequest, HttpServletResponse) line: 897    
    DispatcherServlet(HttpServlet).service(HttpServletRequest, HttpServletResponse) line: 645   
    DispatcherServlet(FrameworkServlet).service(HttpServletRequest, HttpServletResponse) line: 882  
    DispatcherServlet(HttpServlet).service(ServletRequest, ServletResponse) line: 750   
    ServletHandler.handleRequest(HttpServerExchange) line: 74   
    FilterHandler$FilterChainImpl.doFilter(ServletRequest, ServletResponse) line: 129   
    OrderedRequestContextFilter(RequestContextFilter).doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain) line: 99   
    OrderedRequestContextFilter(OncePerRequestFilter).doFilter(ServletRequest, ServletResponse, FilterChain) line: 107  
    ManagedFilter.doFilter(ServletRequest, ServletResponse, FilterChain) line: 61   
    FilterHandler$FilterChainImpl.doFilter(ServletRequest, ServletResponse) line: 131   
    OrderedFormContentFilter(FormContentFilter).doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain) line: 92 
    OrderedFormContentFilter(OncePerRequestFilter).doFilter(ServletRequest, ServletResponse, FilterChain) line: 107 
    ManagedFilter.doFilter(ServletRequest, ServletResponse, FilterChain) line: 61   
    FilterHandler$FilterChainImpl.doFilter(ServletRequest, ServletResponse) line: 131   
    OrderedHiddenHttpMethodFilter(HiddenHttpMethodFilter).doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain) line: 93   
    OrderedHiddenHttpMethodFilter(OncePerRequestFilter).doFilter(ServletRequest, ServletResponse, FilterChain) line: 107    
    ManagedFilter.doFilter(ServletRequest, ServletResponse, FilterChain) line: 61   
    FilterHandler$FilterChainImpl.doFilter(ServletRequest, ServletResponse) line: 131   
    OrderedCharacterEncodingFilter(CharacterEncodingFilter).doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain) line: 200    
    OrderedCharacterEncodingFilter(OncePerRequestFilter).doFilter(ServletRequest, ServletResponse, FilterChain) line: 107   
    ManagedFilter.doFilter(ServletRequest, ServletResponse, FilterChain) line: 61   
    FilterHandler$FilterChainImpl.doFilter(ServletRequest, ServletResponse) line: 131   
    FilterHandler.handleRequest(HttpServerExchange) line: 84    
    ServletSecurityRoleHandler.handleRequest(HttpServerExchange) line: 62   
    ServletChain$1.handleRequest(HttpServerExchange) line: 68   
    ServletDispatchingHandler.handleRequest(HttpServerExchange) line: 36    
    SSLInformationAssociationHandler.handleRequest(HttpServerExchange) line: 132    
    ServletAuthenticationCallHandler.handleRequest(HttpServerExchange) line: 57 
    PredicateHandler.handleRequest(HttpServerExchange) line: 43 
    ServletConfidentialityConstraintHandler(AbstractConfidentialityHandler).handleRequest(HttpServerExchange) line: 46  
    ServletConfidentialityConstraintHandler.handleRequest(HttpServerExchange) line: 64  
    AuthenticationMechanismsHandler.handleRequest(HttpServerExchange) line: 60  
    CachedAuthenticatedSessionHandler.handleRequest(HttpServerExchange) line: 77    
    SecurityInitialHandler(AbstractSecurityContextAssociationHandler).handleRequest(HttpServerExchange) line: 43    
    PredicateHandler.handleRequest(HttpServerExchange) line: 43 
    PredicateHandler.handleRequest(HttpServerExchange) line: 43 
    ServletInitialHandler.handleFirstRequest(HttpServerExchange, ServletRequestContext) line: 292   
    ServletInitialHandler.access$100(ServletInitialHandler, HttpServerExchange, ServletRequestContext) line: 81 
    ServletInitialHandler$2.call(HttpServerExchange, ServletRequestContext) line: 138   
    ServletInitialHandler$2.call(HttpServerExchange, Object) line: 135  
    ServletRequestContextThreadSetupAction$1.call(HttpServerExchange, C) line: 48   
    ContextClassLoaderSetupAction$1.call(HttpServerExchange, C) line: 43    
    ServletInitialHandler.dispatchRequest(HttpServerExchange, ServletRequestContext, ServletChain, DispatcherType) line: 272    
    ServletInitialHandler.access$000(ServletInitialHandler, HttpServerExchange, ServletRequestContext, ServletChain, DispatcherType) line: 81   
    ServletInitialHandler$1.handleRequest(HttpServerExchange) line: 104 
    Connectors.executeRootHandler(HttpHandler, HttpServerExchange) line: 360    
    HttpServerExchange$1.run() line: 830    
    XnioWorker$TaskPool(ThreadPoolExecutor).runWorker(ThreadPoolExecutor$Worker) line: 1149 
    ThreadPoolExecutor$Worker.run() line: 624   
    Thread.run() line: 748  

In the absence of an explicit message, StatusCodes.getReason(sc) is used. If instead sendError(int, String) was called directly, the supplied message would have been used. In short, you can achieve the behaviour that you want on Undertow by supplying a message when you call sendError in your exception handler:

  @ExceptionHandler(IllegalArgumentException.class)
  public void illegalArgument(IllegalArgumentException ex, HttpServletResponse response) throws IOException {
    response.sendError(HttpStatus.BAD_REQUEST.value(), ex.getMessage());
  }
pperez commented 5 years ago

Thanks Andy, as this code is part of spring cloud config's vault integration i'll report it to them