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

Connection is closed when sending a file causes NullPointerException #80

Closed
gjroylance opened this Issue Jan 5, 2016 · 8 comments

Comments

5 participants
@gjroylance

gjroylance commented Jan 5, 2016

NullPointerException
io.vertx.core.http.impl.HttpServerResponseImpl in doSendFile at line 483
io.vertx.core.http.impl.HttpServerResponseImpl in sendFile at line 339
io.vertx.core.http.HttpServerResponse in sendFile at line 316
io.vertx.core.http.HttpServerResponse in sendFile at line 302
io.vertx.rxjava.core.http.HttpServerResponse in sendFile at line 334

@RuchiraKulathunga

This comment has been minimized.

Show comment
Hide comment
@RuchiraKulathunga

RuchiraKulathunga Jul 13, 2016

There is this security vulnerability and I am working on this issue..

RuchiraKulathunga commented Jul 13, 2016

There is this security vulnerability and I am working on this issue..

@woollybah

This comment has been minimized.

Show comment
Hide comment
@woollybah

woollybah Sep 15, 2016

I'm also seeing this in 3.3.2

java.lang.NullPointerException
    at io.vertx.core.http.impl.HttpServerResponseImpl.doSendFile(HttpServerResponseImpl.java:491)
    at io.vertx.core.http.impl.HttpServerResponseImpl.sendFile(HttpServerResponseImpl.java:342)
    at io.vertx.core.http.HttpServerResponse.sendFile(HttpServerResponse.java:317)
    at io.vertx.core.http.HttpServerResponse.sendFile(HttpServerResponse.java:303)

woollybah commented Sep 15, 2016

I'm also seeing this in 3.3.2

java.lang.NullPointerException
    at io.vertx.core.http.impl.HttpServerResponseImpl.doSendFile(HttpServerResponseImpl.java:491)
    at io.vertx.core.http.impl.HttpServerResponseImpl.sendFile(HttpServerResponseImpl.java:342)
    at io.vertx.core.http.HttpServerResponse.sendFile(HttpServerResponse.java:317)
    at io.vertx.core.http.HttpServerResponse.sendFile(HttpServerResponse.java:303)
@vietj

This comment has been minimized.

Show comment
Hide comment
@vietj

vietj Sep 15, 2016

@woollybah could you provide a reproducer ?

vietj commented Sep 15, 2016

@woollybah could you provide a reproducer ?

@woollybah

This comment has been minimized.

Show comment
Hide comment
@woollybah

woollybah Sep 15, 2016

After further testing, I have found that my particular problem concerns this "double-tap" issue in chrome ( https://bugs.chromium.org/p/chromium/issues/detail?id=587709 ), which relates to chrome sending two requests to the server for a PDF.

It appears that one of the channels is closed by chrome before completion, and it is this that vertx doesn't handle very well, resulting in ServerConnection writeToChannel() returning null on one of two the requests.
The other request succeeds, and file retrieval is completed.

woollybah commented Sep 15, 2016

After further testing, I have found that my particular problem concerns this "double-tap" issue in chrome ( https://bugs.chromium.org/p/chromium/issues/detail?id=587709 ), which relates to chrome sending two requests to the server for a PDF.

It appears that one of the channels is closed by chrome before completion, and it is this that vertx doesn't handle very well, resulting in ServerConnection writeToChannel() returning null on one of two the requests.
The other request succeeds, and file retrieval is completed.

@pmlopes

This comment has been minimized.

Show comment
Hide comment
@pmlopes

pmlopes Sep 15, 2016

Member

@woollybah if the client closes the connection then an exception is expected on the server side right. So what you would like to see is that the exception is thrown from writeToChannel instead of a NPE?

Member

pmlopes commented Sep 15, 2016

@woollybah if the client closes the connection then an exception is expected on the server side right. So what you would like to see is that the exception is thrown from writeToChannel instead of a NPE?

@woollybah

This comment has been minimized.

Show comment
Hide comment
@woollybah

woollybah Sep 15, 2016

Well, what I'm not expecting is for HttpServerResponse sendFile() to throw a NullPointerException.
It would be nice if the exception (whatever you decide it to be) was contained within the resultHandler, so I can handle it normally.
Thanks!

woollybah commented Sep 15, 2016

Well, what I'm not expecting is for HttpServerResponse sendFile() to throw a NullPointerException.
It would be nice if the exception (whatever you decide it to be) was contained within the resultHandler, so I can handle it normally.
Thanks!

@vietj

This comment has been minimized.

Show comment
Hide comment
@vietj

vietj Sep 15, 2016

+1 @woollybah however we would like to have a clear reproducer of the problem that would help a lot :-)

vietj commented Sep 15, 2016

+1 @woollybah however we would like to have a clear reproducer of the problem that would help a lot :-)

@woollybah

This comment has been minimized.

Show comment
Hide comment
@woollybah

woollybah Oct 18, 2016

Here's something that demonstrates the issue...

https://github.com/woollybah/sendfile-npe-reproducer

Open the page in Chrome, and then you can basically just hold-down ctrl-R to refresh the page lots.
I get the exception almost immediately here.

woollybah commented Oct 18, 2016

Here's something that demonstrates the issue...

https://github.com/woollybah/sendfile-npe-reproducer

Open the page in Chrome, and then you can basically just hold-down ctrl-R to refresh the page lots.
I get the exception almost immediately here.

pmlopes added a commit to eclipse-vertx/vert.x that referenced this issue May 10, 2017

Fixes vert-x3/issues#80: Avoid NPE and call handler with Failed Future
Signed-off-by: Paulo Lopes <pmlopes@gmail.com>

pmlopes added a commit to eclipse-vertx/vert.x that referenced this issue May 11, 2017

Add test showing the premature kill of the connection vert-x3/issues#80
Signed-off-by: Paulo Lopes <pmlopes@gmail.com>

pmlopes added a commit to eclipse-vertx/vert.x that referenced this issue May 11, 2017

Add test showing the premature kill of the connection vert-x3/issues#80
Signed-off-by: Paulo Lopes <pmlopes@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment