Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NPE occur when request body is empty. (TomcatService) #410

Closed
be-hase opened this issue Feb 21, 2017 · 2 comments
Closed

NPE occur when request body is empty. (TomcatService) #410

be-hase opened this issue Feb 21, 2017 · 2 comments
Assignees
Labels
Milestone

Comments

@be-hase
Copy link
Member

be-hase commented Feb 21, 2017

Hi.

I am using armeria-tomcat with spring-mvc.
For example, controller is like the following code.

@RestController
public static class MyController {

    @PostMapping("/hoge")
    public String hoge(
            @RequestBody String body
    ) {
        return "POST";
    }
}

NPE occur when request body is empty.
Stack trace is like the following.

java.lang.NullPointerException: null
        at org.apache.coyote.Request.doRead(Request.java:511)
        at org.apache.catalina.connector.InputBuffer.realReadBytes(InputBuffer.java:318)
        at org.apache.tomcat.util.buf.ByteChunk.checkEof(ByteChunk.java:397)
        at org.apache.tomcat.util.buf.ByteChunk.substract(ByteChunk.java:363)
        at org.apache.catalina.connector.InputBuffer.readByte(InputBuffer.java:329)
        at org.apache.catalina.connector.CoyoteInputStream.read(CoyoteInputStream.java:93)
        at java.io.FilterInputStream.read(FilterInputStream.java:83)
        at java.io.PushbackInputStream.read(PushbackInputStream.java:139)
        at org.springframework.web.servlet.mvc.method.annotation.AbstractMessageConverterMethodArgumentResolver$EmptyBodyCheckingHttpInputMessage.<init>(AbstractMessageConverterMethodArgumentResolver.java:311)
        at org.springframework.web.servlet.mvc.method.annotation.AbstractMessageConverterMethodArgumentResolver.readWithMessageConverters(AbstractMessageConverterMethodArgumentResolver.java:185)
        at org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor.readWithMessageConverters(RequestResponseBodyMethodProcessor.java:149)
        at org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor.resolveArgument(RequestResponseBodyMethodProcessor.java:127)
        at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:121)
        at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:160)
        at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:129)
        at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:116)
        at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:827)
        at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:738)
        at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:85)
        at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:963)
        at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:897)
        at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970)
        at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:872)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
        at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:846)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:230)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:165)
        at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:192)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:165)
        at org.springframework.session.web.http.SessionRepositoryFilter.doFilterInternal(SessionRepositoryFilter.java:164)
        at org.springframework.session.web.http.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:80)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:192)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:165)
        at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:197)
        at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:192)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:165)
        at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:198)
        at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:108)
        at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:472)
        at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:140)
        at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
        at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
        at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:349)
        at com.linecorp.armeria.server.http.tomcat.TomcatService.lambda$null$4(TomcatService.java:407)
        at com.linecorp.armeria.common.AbstractRequestContext.lambda$makeContextAware$1(AbstractRequestContext.java:59)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:144)
        at java.lang.Thread.run(Thread.java:745)

It seems to be null because it is not set here when content is empty.

https://github.com/line/armeria/blob/master/tomcat/src/main/java/com/linecorp/armeria/server/http/tomcat/TomcatService.java#L467-L469

Can I send PR ?

@@ -464,9 +464,7 @@ public final class TomcatService implements HttpService {
 
         // Set the content.
         final HttpData content = req.content();
-        if (!content.isEmpty()) {
-            coyoteReq.setInputBuffer(new InputBufferImpl(content));
-        }
+        coyoteReq.setInputBuffer(new InputBufferImpl(content));
 
         return coyoteReq;
     }
@@ -574,7 +572,7 @@ public final class TomcatService implements HttpService {
 
         @Override
         public int doRead(ByteChunk chunk) {
-            if (read) {
+            if (read || content.isEmpty()) {
                 // Read only once.
                 return -1;
             }

@trustin trustin added the defect label Feb 21, 2017
@trustin
Copy link
Member

trustin commented Feb 21, 2017

@be-hase The proposed fix sounds good to me. Yes, please send a PR! Thanks. 👍

@trustin trustin added this to the 0.38.0 milestone Feb 21, 2017
be-hase added a commit to be-hase/armeria that referenced this issue Feb 21, 2017
@be-hase
Copy link
Member Author

be-hase commented Feb 21, 2017

@trustin
thanks.
I will send the PR.

be-hase added a commit to be-hase/armeria that referenced this issue Feb 22, 2017
trustin pushed a commit that referenced this issue Feb 22, 2017
This commit fixes a bug where an empty POST body triggers a NullPointerException.
@trustin trustin self-assigned this Feb 22, 2017
@trustin trustin closed this as completed Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants