Skip to content

Commit

Permalink
[Bug]Fix internal server error when Bookkeeper HTTP services are sent…
Browse files Browse the repository at this point in the history
… invalid values in json input
  • Loading branch information
Apurva Telang committed Apr 18, 2023
1 parent 94e15b3 commit c034c22
Show file tree
Hide file tree
Showing 9 changed files with 422 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,16 @@
import org.apache.bookkeeper.http.service.HttpEndpointService;
import org.apache.bookkeeper.http.service.HttpServiceRequest;
import org.apache.bookkeeper.http.service.HttpServiceResponse;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Http Handler for Vertx based Http Server.
*/
public abstract class VertxAbstractHandler implements Handler<RoutingContext> {

private static final Logger LOG = LoggerFactory.getLogger(VertxAbstractHandler.class);

/**
* Process the request using the given httpEndpointService.
*/
Expand All @@ -53,6 +57,7 @@ void processRequest(HttpEndpointService httpEndpointService, RoutingContext cont
try {
response = httpEndpointService.handle(request);
} catch (Exception e) {
LOG.error("Failed to execute http request:", e);
response = new ErrorHttpService().handle(request);
}
httpResponse.setStatusCode(response.getStatusCode());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,25 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
} else {
@SuppressWarnings("unchecked")
Map<String, Object> configMap = JsonUtil.fromJson(requestBody, HashMap.class);
Boolean resumeMajor = (Boolean) configMap.get("resumeMajor");
Boolean resumeMinor = (Boolean) configMap.get("resumeMinor");
if (resumeMajor == null && resumeMinor == null) {
Object resumeMajorObj = configMap.get("resumeMajor");
Object resumeMinorObj = configMap.get("resumeMinor");
boolean resumeMajor = false, resumeMinor = false;
if (resumeMajorObj instanceof Boolean) {
resumeMajor = (Boolean) resumeMajorObj;
}
if (resumeMinorObj instanceof Boolean) {
resumeMinor = (Boolean) resumeMinorObj;
}
if (!resumeMajor && !resumeMinor) {
return new HttpServiceResponse("No resumeMajor or resumeMinor params found",
HttpServer.StatusCode.BAD_REQUEST);
}
String output = "";
if (resumeMajor != null && resumeMajor) {
if (resumeMajor) {
output = "Resume majorGC on BookieServer: " + bookieServer.toString();
bookieServer.getBookie().getLedgerStorage().resumeMajorGC();
}
if (resumeMinor != null && resumeMinor) {
if (resumeMinor) {
output += ", Resume minorGC on BookieServer: " + bookieServer.toString();
bookieServer.getBookie().getLedgerStorage().resumeMinorGC();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,26 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
} else {
@SuppressWarnings("unchecked")
Map<String, Object> configMap = JsonUtil.fromJson(requestBody, HashMap.class);
Boolean suspendMajor = (Boolean) configMap.get("suspendMajor");
Boolean suspendMinor = (Boolean) configMap.get("suspendMinor");
if (suspendMajor == null && suspendMinor == null) {
Object suspendMajorObj = configMap.get("suspendMajor");
Object suspendMinorObj = configMap.get("suspendMinor");
boolean suspendMajor = false, suspendMinor = false;
if (suspendMajorObj instanceof Boolean) {
suspendMajor = (Boolean) suspendMajorObj;
}
if (suspendMinorObj instanceof Boolean) {
suspendMinor = (Boolean) suspendMinorObj;
}

if (!suspendMajor && !suspendMinor) {
return new HttpServiceResponse("No suspendMajor or suspendMinor params found",
HttpServer.StatusCode.BAD_REQUEST);
}
String output = "";
if (suspendMajor != null && suspendMajor) {
if (suspendMajor) {
output = "Suspend majorGC on BookieServer: " + bookieServer.toString();
bookieServer.getBookie().getLedgerStorage().suspendMajorGC();
}
if (suspendMinor != null && suspendMinor) {
if (suspendMinor) {
output += ", Suspend minorGC on BookieServer: " + bookieServer.toString();
bookieServer.getBookie().getLedgerStorage().suspendMinorGC();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,15 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
} else {
@SuppressWarnings("unchecked")
Map<String, Object> configMap = JsonUtil.fromJson(requestBody, HashMap.class);
Boolean forceMajor = (Boolean) configMap.getOrDefault("forceMajor", null);
Boolean forceMinor = (Boolean) configMap.getOrDefault("forceMinor", null);
Object forceMajorObj = configMap.getOrDefault("forceMajor", null);
Object forceMinorObj = configMap.getOrDefault("forceMinor", null);
boolean forceMajor = false, forceMinor = false;
if (forceMajorObj instanceof Boolean) {
forceMajor = (Boolean) forceMajorObj;
}
if (forceMinorObj instanceof Boolean) {
forceMinor = (Boolean) forceMinorObj;
}
bookieServer.getBookie().getLedgerStorage().forceGC(forceMajor, forceMinor);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,17 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
try {
@SuppressWarnings("unchecked")
Map<String, Object> configMap = JsonUtil.fromJson(requestBody, HashMap.class);
Boolean isEntryLocationCompact = (Boolean) configMap
.getOrDefault("entryLocationRocksDBCompact", false);
String entryLocations = (String) configMap.getOrDefault("entryLocations", "");
Object isEntryLocationCompactObj = configMap
.get("entryLocationRocksDBCompact");
boolean isEntryLocationCompact = false;
if (isEntryLocationCompactObj instanceof Boolean) {
isEntryLocationCompact = (Boolean) isEntryLocationCompactObj;
}
Object entryLocationsObj = configMap.get("entryLocations");
String entryLocations = "";
if (entryLocationsObj instanceof String) {
entryLocations = (String) entryLocationsObj;
}

if (!isEntryLocationCompact) {
// If entryLocationRocksDBCompact is false, doing nothing.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.bookkeeper.server.http.service;

import static org.junit.Assert.assertEquals;

import java.util.Map;
import org.apache.bookkeeper.common.util.JsonUtil;
import org.apache.bookkeeper.http.HttpServer;
import org.apache.bookkeeper.http.service.HttpServiceRequest;
import org.apache.bookkeeper.http.service.HttpServiceResponse;
import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
import org.junit.Before;
import org.junit.Test;

public class ResumeCompactionServiceTest extends BookKeeperClusterTestCase {

private static final int numberOfBookies = 1;
private SuspendCompactionService suspendCompactionService;
private ResumeCompactionService resumeCompactionService;

public ResumeCompactionServiceTest() {
super(numberOfBookies);
}

@Override
@Before
public void setUp() throws Exception {
super.setUp();
resumeCompactionService = new ResumeCompactionService(serverByIndex(numBookies - 1));
suspendCompactionService = new SuspendCompactionService(serverByIndex(numBookies - 1));
}

@Test
public void testResumeCompactionWithBadMajorAndMinorCompactArgs() throws Exception {
String triggerResumeBody = "{ \"resumeMajor\": \"true\", \"resumeMinor\": \"true\" }";
HttpServiceRequest resumeRequest = new HttpServiceRequest(triggerResumeBody, HttpServer.Method.PUT, null);
HttpServiceResponse response = resumeCompactionService.handle(resumeRequest);
assertEquals(HttpServer.StatusCode.BAD_REQUEST.getValue(), response.getStatusCode());
}

@Test
public void testResumeCompactionWithBadMajorAndGoodMinorCompactArgs() throws Exception {
String triggerSuspendBody = "{ \"suspendMajor\": true, \"suspendMinor\": true }";
String triggerResumeBody = "{ \"resumeMajor\": \"true\", \"resumeMinor\": true }";
HttpServiceRequest suspendRequest = new HttpServiceRequest(triggerSuspendBody, HttpServer.Method.PUT, null);
suspendCompactionService.handle(suspendRequest);
Map beforeResumeStatusResponseMap = getCompactionStatus();
assertEquals("true", beforeResumeStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("true", beforeResumeStatusResponseMap.get("isMinorGcSuspended"));
HttpServiceRequest resumeRequest = new HttpServiceRequest(triggerResumeBody, HttpServer.Method.PUT, null);
HttpServiceResponse response = resumeCompactionService.handle(resumeRequest);
assertEquals(HttpServer.StatusCode.OK.getValue(), response.getStatusCode());
Map afterResumeStatusResponseMap = getCompactionStatus();
assertEquals("true", afterResumeStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("false", afterResumeStatusResponseMap.get("isMinorGcSuspended"));
}

@Test
public void testResumeCompactionWithGoodMajorAndBadMinorCompactArgs() throws Exception {
String triggerSuspendBody = "{ \"suspendMajor\": true, \"suspendMinor\": true }";
String triggerResumeBody = "{ \"resumeMajor\": true, \"resumeMinor\": \"true\" }";
HttpServiceRequest suspendRequest = new HttpServiceRequest(triggerSuspendBody, HttpServer.Method.PUT, null);
suspendCompactionService.handle(suspendRequest);
Map beforeResumeStatusResponseMap = getCompactionStatus();
assertEquals("true", beforeResumeStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("true", beforeResumeStatusResponseMap.get("isMinorGcSuspended"));
HttpServiceRequest resumeRequest = new HttpServiceRequest(triggerResumeBody, HttpServer.Method.PUT, null);
HttpServiceResponse response = resumeCompactionService.handle(resumeRequest);
assertEquals(HttpServer.StatusCode.OK.getValue(), response.getStatusCode());
Map afterResumeStatusResponseMap = getCompactionStatus();
assertEquals("false", afterResumeStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("true", afterResumeStatusResponseMap.get("isMinorGcSuspended"));
}

private Map getCompactionStatus() throws Exception {
HttpServiceRequest statusRequest = new HttpServiceRequest(null, HttpServer.Method.GET, null);
HttpServiceResponse statusResponse = suspendCompactionService.handle(statusRequest);
return JsonUtil.fromJson(statusResponse.getBody(), Map.class);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.bookkeeper.server.http.service;

import static org.junit.Assert.assertEquals;

import java.util.Map;
import org.apache.bookkeeper.common.util.JsonUtil;
import org.apache.bookkeeper.http.HttpServer;
import org.apache.bookkeeper.http.service.HttpServiceRequest;
import org.apache.bookkeeper.http.service.HttpServiceResponse;
import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
import org.junit.Before;
import org.junit.Test;

public class SuspendCompactionServiceTest extends BookKeeperClusterTestCase {

private static final int numberOfBookies = 1;
private SuspendCompactionService suspendCompactionService;

public SuspendCompactionServiceTest() {
super(numberOfBookies);
}

@Override
@Before
public void setUp() throws Exception {
super.setUp();
suspendCompactionService = new SuspendCompactionService(serverByIndex(numBookies - 1));
}

@Test
public void testSuspendCompactionWithBadMajorAndMinorCompactArgs() throws Exception {
String testBody = "{ \"suspendMajor\": \"true\", \"suspendMinor\": \"true\" }";
Map beforeStatusResponseMap = getCompactionStatus();
assertEquals("false", beforeStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("false", beforeStatusResponseMap.get("isMinorGcSuspended"));
HttpServiceRequest suspendRequest = new HttpServiceRequest(testBody, HttpServer.Method.PUT, null);
HttpServiceResponse response = suspendCompactionService.handle(suspendRequest);
assertEquals(HttpServer.StatusCode.BAD_REQUEST.getValue(), response.getStatusCode());

}

@Test
public void testSuspendCompactionWithBadMajorAndGoodMinorCompactArgs() throws Exception {
String testBody = "{ \"suspendMajor\": \"true\", \"suspendMinor\": true }";
Map beforeStatusResponseMap = getCompactionStatus();
assertEquals("false", beforeStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("false", beforeStatusResponseMap.get("isMinorGcSuspended"));
HttpServiceRequest suspendRequest = new HttpServiceRequest(testBody, HttpServer.Method.PUT, null);
HttpServiceResponse response = suspendCompactionService.handle(suspendRequest);
assertEquals(HttpServer.StatusCode.OK.getValue(), response.getStatusCode());
Map afterStatusResponseMap = getCompactionStatus();
assertEquals("false", afterStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("true", afterStatusResponseMap.get("isMinorGcSuspended"));
}

@Test
public void testSuspendCompactionWithGoodMajorAndBadMinorCompactArgs() throws Exception {
String testBody = "{ \"suspendMajor\": true, \"suspendMinor\": \"true\" }";
Map beforeStatusResponseMap = getCompactionStatus();
assertEquals("false", beforeStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("false", beforeStatusResponseMap.get("isMinorGcSuspended"));
HttpServiceRequest suspendRequest = new HttpServiceRequest(testBody, HttpServer.Method.PUT, null);
HttpServiceResponse response = suspendCompactionService.handle(suspendRequest);
assertEquals(HttpServer.StatusCode.OK.getValue(), response.getStatusCode());
Map afterStatusResponseMap = getCompactionStatus();
assertEquals("true", afterStatusResponseMap.get("isMajorGcSuspended"));
assertEquals("false", afterStatusResponseMap.get("isMinorGcSuspended"));
}

private Map getCompactionStatus() throws Exception {
HttpServiceRequest statusRequest = new HttpServiceRequest(null, HttpServer.Method.GET, null);
HttpServiceResponse statusResponse = suspendCompactionService.handle(statusRequest);
return JsonUtil.fromJson(statusResponse.getBody(), Map.class);
}

}
Loading

0 comments on commit c034c22

Please sign in to comment.