Skip to content

Commit

Permalink
null value in HttpRequest.sendJson and HttpRequest.sendJsonable shoul…
Browse files Browse the repository at this point in the history
…d be treated as json null - fixes #1436
  • Loading branch information
vietj committed Oct 25, 2019
1 parent bd7d5f2 commit de88aaa
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
Expand Up @@ -430,15 +430,15 @@ private void handleSendRequest() {
if (request.timeout > 0) {
req.setTimeout(request.timeout);
}
if (body != null) {
if (contentType != null) {
String prev = req.headers().get(HttpHeaders.CONTENT_TYPE);
if (prev == null) {
req.putHeader(HttpHeaders.CONTENT_TYPE, contentType);
} else {
contentType = prev;
}
if (contentType != null) {
String prev = req.headers().get(HttpHeaders.CONTENT_TYPE);
if (prev == null) {
req.putHeader(HttpHeaders.CONTENT_TYPE, contentType);
} else {
contentType = prev;
}
}
if (body != null || "application/json".equals(contentType)) {

This comment has been minimized.

Copy link
@gagarski

gagarski Oct 25, 2019

Now NPE can happen in line 486 (for JsonObject instance). BTW is that branch (else if (body instanceof JsonObject)) needed at all? Won't Json.encode(body) handle this case?

This comment has been minimized.

Copy link
@vietj

vietj Oct 25, 2019

Author Contributor

(null instanceof JsonObject) is always evaluated to false. Indeed I think we could get rid of this un-necessary statement though

This comment has been minimized.

Copy link
@vietj

vietj Oct 25, 2019

Author Contributor

At least for Vert.x 4, I am not sure it will work for Vert.x 3.x

This comment has been minimized.

Copy link
@gagarski

gagarski Oct 25, 2019

(null instanceof JsonObject) is always evaluated to false.

Yeah, my mistake, sorry.

I am not sure it will work for Vert.x 3.x

Well, It needs checking but I cannot see why not: JsonObject::encode uses Json.mapper under the hood and Json.mapper can serialize JsonObject as JSON (a module that allows it is added in static init block of Json class)

This comment has been minimized.

Copy link
@vietj

vietj via email Oct 26, 2019

Author Contributor
if (body instanceof MultiMap) {
MultipartForm parts = MultipartForm.create();
MultiMap attributes = (MultiMap) body;
Expand Down
Expand Up @@ -305,6 +305,14 @@ public void testSendBufferBody() throws Exception {
testSendBody(body, (contentType, buff) -> assertEquals(body, buff));
}

@Test
public void testSendJsonNullBody() throws Exception {
testSendBody(null, (contentType, buff) -> {
assertEquals("application/json", contentType);
// assertEquals(body, buff.toJsonObject());
});
}

private void testSendBody(Object body, BiConsumer<String, Buffer> checker) throws Exception {
waitFor(2);
server.requestHandler(req -> req.bodyHandler(buff -> {
Expand Down

0 comments on commit de88aaa

Please sign in to comment.