search

USACE / cwms-data-api

Corps Water Management System RESTful Data Service
MIT License
13 stars 14 forks source link

NullPointerException on header parse #272

Closed adamkorynta closed 1 year ago

adamkorynta commented 2 years ago

I'm running into the following issue client side: https://github.com/square/okhttp/issues/2099, where I tell OkHttp to use application/json;version=2, but then the OkHttp client library converts this to application/json;version=2; charset=utf-8.

This ends up throwing the following exception server-side:

07-Nov-2022 19:35:38.693 WARNING [https-jsse-nio-8443-exec-3] cwms.radar.ApiServlet.lambda$init$10 error on request[3357625609381316046]: /cwms-data/timeseries
    java.lang.NullPointerException
        at cwms.radar.api.TimeSeriesController.deserializeTimeSeries(TimeSeriesController.java:400)
        at cwms.radar.api.TimeSeriesController.deserializeTimeSeries(TimeSeriesController.java:395)
        at cwms.radar.api.TimeSeriesController.create(TimeSeriesController.java:127)
        at io.javalin.apibuilder.CrudFunction$3.invoke$lambda-0(CrudHandler.kt:32)
        at io.javalin.apibuilder.CrudFunctionHandler.handle(CrudHandler.kt)
        at cwms.radar.security.CwmsAccessManager.manage(CwmsAccessManager.java:42)
        at io.javalin.http.JavalinServlet.addHandler$lambda-0(JavalinServlet.kt:96)
        at io.javalin.http.JavalinServlet$lifecycle$2$1$1.invoke(JavalinServlet.kt:43)
        at io.javalin.http.JavalinServlet$lifecycle$2$1$1.invoke(JavalinServlet.kt:43)
        at io.javalin.http.JavalinServletHandler.executeNextTask(JavalinServletHandler.kt:99)
        at io.javalin.http.JavalinServletHandler.queueNextTaskOrFinish$lambda-1(JavalinServletHandler.kt:85)
        at java.util.concurrent.CompletableFuture.uniComposeStage(CompletableFuture.java:995)
        at java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:2137)
        at io.javalin.http.JavalinServletHandler.queueNextTaskOrFinish$javalin(JavalinServletHandler.kt:85)
        at io.javalin.http.JavalinServletHandler.executeNextTask$lambda-11$lambda-10(JavalinServletHandler.kt:119)
        at java.util.concurrent.CompletableFuture.uniApply(CompletableFuture.java:616)
        at java.util.concurrent.CompletableFuture.uniApplyStage(CompletableFuture.java:628)
        at java.util.concurrent.CompletableFuture.thenApply(CompletableFuture.java:1996)
        at io.javalin.http.JavalinServletHandler.executeNextTask(JavalinServletHandler.kt:119)
        at io.javalin.http.JavalinServletHandler.queueNextTaskOrFinish$lambda-1(JavalinServletHandler.kt:85)
        at java.util.concurrent.CompletableFuture.uniComposeStage(CompletableFuture.java:995)
        at java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:2137)
        at io.javalin.http.JavalinServletHandler.queueNextTaskOrFinish$javalin(JavalinServletHandler.kt:85)
        at io.javalin.http.JavalinServlet.service(JavalinServlet.kt:89)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:779)
        at cwms.radar.ApiServlet.service(ApiServlet.java:343)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:779)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:227)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
        at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
        at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:197)
        at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:97)
        at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:660)
        at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:135)
        at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
        at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:687)
        at org.apache.catalina.authenticator.SingleSignOn.invoke(SingleSignOn.java:312)
        at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78)
        at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:360)
        at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:399)
        at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
        at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:893)
        at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1789)
        at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
        at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)
        at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
        at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
        at java.lang.Thread.run(Thread.java:750)

Reading this post (though I'm far from knowledge on the subject to vet the answer) this format looks valid: https://stackoverflow.com/questions/29549299/how-to-split-header-values

rma-rripken commented 2 years ago

The problem comes down to Formats.parseHeader. That method is often used on the accept header but its also used on the ContentType for POST, PUT and UPDATE.

Maybe we shouldn't be reusing the method and should have a different method for ContentType ? Not sure.

Maybe we should be looking for charset explicitly and handling it somehow and if its been handled removing it from the ContentType string? Again, not sure.

Or, maybe Formats.parseHeader shouldn't be doing contentTypeList.contains(ct) but should be using some other method for finding a match in our list of supported ContentTypes.

It looks like our handling of asterisk in the ContentTypes isn't doing what I think asterisk is supposed to be doing.

Additional unit tests around Formats.parseHeader would probably help clarify what its doing and what it should be doing.

MikeNeilson commented 1 year ago

This will be fixed with #238, the same situation (but RestAssured instead of OkHttp) is happening. I've tweak the equals function to only care about the mimetype and version parameter.

rma-rripken commented 1 year ago

I made a change to ContentType that may also have fixed this issue. Charset now gets handled on its own so it doesn't end up in the list of ContentType parameters.

MikeNeilson commented 1 year ago

Okay, I think I picked that up along with what I was doing. though I just realize that may particular issue wasn't in the ContentType class, is was string comparisons in ClobController.

But there can be other parameters so isolating to just the one ones we truly care about is probably the better thing to do.