Skip to content

Commit 7a65e45

Browse files
pradeepagrawal8184pradeepagrawal8184
committed
RANGER-4076: Addressed review comment of Vyom
1 parent 331a092 commit 7a65e45

6 files changed

Lines changed: 510 additions & 38 deletions

File tree

‎agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRESTClient.java‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,23 @@
7070
public class RangerRESTClient {
7171
private static final Logger LOG = LoggerFactory.getLogger(RangerRESTClient.class);
7272

73+
public enum HttpMethod {
74+
GET("GET"),
75+
POST("POST"),
76+
PUT("PUT"),
77+
DELETE("DELETE");
78+
79+
private final String method;
80+
81+
HttpMethod(String method) {
82+
this.method = method;
83+
}
84+
85+
public String getMethod() {
86+
return method;
87+
}
88+
}
89+
7390
public static final String RANGER_PROP_POLICYMGR_URL = "ranger.service.store.rest.url";
7491
public static final String RANGER_PROP_POLICYMGR_SSLCONFIG_FILENAME = "ranger.service.store.rest.ssl.config.file";
7592
public static final String RANGER_POLICYMGR_CLIENT_KEY_FILE = "xasecure.policymgr.clientssl.keystore";
@@ -367,35 +384,35 @@ public TrustManager[] getTrustManagers(String trustStoreFile, String trustStoreF
367384
}
368385

369386
public Response get(String relativeUrl, Map<String, String> params) throws Exception {
370-
return performRequest("GET", relativeUrl, params, null, null);
387+
return performRequest(HttpMethod.GET, relativeUrl, params, null, null);
371388
}
372389

373390
public Response get(String relativeUrl, Map<String, String> params, Cookie sessionId) throws Exception {
374-
return performRequest("GET", relativeUrl, params, null, sessionId);
391+
return performRequest(HttpMethod.GET, relativeUrl, params, null, sessionId);
375392
}
376393

377394
public Response post(String relativeUrl, Map<String, String> params, Object obj) throws Exception {
378-
return performRequest("POST", relativeUrl, params, obj, null);
395+
return performRequest(HttpMethod.POST, relativeUrl, params, obj, null);
379396
}
380397

381398
public Response post(String relativeURL, Map<String, String> params, Object obj, Cookie sessionId) throws Exception {
382-
return performRequest("POST", relativeURL, params, obj, sessionId);
399+
return performRequest(HttpMethod.POST, relativeURL, params, obj, sessionId);
383400
}
384401

385402
public Response delete(String relativeUrl, Map<String, String> params) throws Exception {
386-
return performRequest("DELETE", relativeUrl, params, null, null);
403+
return performRequest(HttpMethod.DELETE, relativeUrl, params, null, null);
387404
}
388405

389406
public Response delete(String relativeURL, Map<String, String> params, Cookie sessionId) throws Exception {
390-
return performRequest("DELETE", relativeURL, params, null, sessionId);
407+
return performRequest(HttpMethod.DELETE, relativeURL, params, null, sessionId);
391408
}
392409

393410
public Response put(String relativeUrl, Map<String, String> params, Object obj) throws Exception {
394-
return performRequest("PUT", relativeUrl, params, obj, null);
411+
return performRequest(HttpMethod.PUT, relativeUrl, params, obj, null);
395412
}
396413

397414
public Response put(String relativeURL, Object request, Cookie sessionId) throws Exception {
398-
return performRequest("PUT", relativeURL, null, request, sessionId);
415+
return performRequest(HttpMethod.PUT, relativeURL, null, request, sessionId);
399416
}
400417

401418
public int getLastKnownActiveUrlIndex() {
@@ -481,7 +498,7 @@ private Invocation.Builder createInvocationBuilder(int currentIndex, String rela
481498
return builder;
482499
}
483500

484-
private Response performRequest(String method, String relativeUrl, Map<String, String> params, Object requestBody, Cookie sessionId) throws Exception {
501+
private Response performRequest(HttpMethod method, String relativeUrl, Map<String, String> params, Object requestBody, Cookie sessionId) throws Exception {
485502
Response finalResponse = null;
486503
int startIndex = this.lastKnownActiveUrlIndex;
487504
int retryAttempt = 0;
@@ -492,11 +509,11 @@ private Response performRequest(String method, String relativeUrl, Map<String, S
492509
try {
493510
Invocation.Builder builder = createInvocationBuilder(currentIndex, relativeUrl, params, sessionId);
494511

495-
if ("POST".equalsIgnoreCase(method)) {
512+
if (HttpMethod.POST == method) {
496513
finalResponse = builder.post(Entity.entity(requestBody, MediaType.APPLICATION_JSON));
497-
} else if ("PUT".equalsIgnoreCase(method)) {
514+
} else if (HttpMethod.PUT == method) {
498515
finalResponse = builder.put(Entity.entity(requestBody, MediaType.APPLICATION_JSON));
499-
} else if ("DELETE".equalsIgnoreCase(method)) {
516+
} else if (HttpMethod.DELETE == method) {
500517
finalResponse = builder.delete();
501518
} else {
502519
finalResponse = builder.get();
@@ -722,12 +739,9 @@ private boolean isSslEnabled(String url) {
722739
}
723740

724741
private KeyManager[] getKeyManagers() {
725-
KeyManager[] kmList = null;
726-
727742
String keyStoreFilepwd = getCredential(mKeyStoreURL, mKeyStoreAlias);
728743

729-
kmList = getKeyManagers(mKeyStoreFile, keyStoreFilepwd);
730-
return kmList;
744+
return getKeyManagers(mKeyStoreFile, keyStoreFilepwd);
731745
}
732746

733747
private TrustManager[] getTrustManagers() {

‎agents-common/src/test/java/org/apache/ranger/admin/client/datatype/TestRESTResponse.java‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,16 @@ public void test03_getMessage_usesMsgDescElseHttpStatus() {
7878
}
7979

8080
@Test
81-
public void test04_fromClientResponse_parsesBodyAndSetsStatus() throws Exception {
82-
// Test with a null response to ensure default behavior works
83-
RESTResponse nullResp = RESTResponse.fromClientResponse(null);
84-
assertNotNull(nullResp);
85-
assertEquals(0, nullResp.getHttpStatusCode());
86-
87-
// Test with a valid JSON string directly
88-
String jsonResponse = "{\"statusCode\":0,\"msgDesc\":\"done\"}";
89-
RESTResponse fromJson = RESTResponse.fromJson(jsonResponse);
90-
assertNotNull(fromJson);
91-
assertEquals(0, fromJson.getStatusCode());
92-
assertEquals("done", fromJson.getMsgDesc());
81+
public void test04_fromClientResponse_parsesBodyAndSetsStatus() {
82+
Response clientResponse = mock(Response.class);
83+
when(clientResponse.getStatus()).thenReturn(201);
84+
when(clientResponse.readEntity(String.class)).thenReturn("{\"statusCode\":0,\"msgDesc\":\"done\"}");
85+
86+
RESTResponse resp = RESTResponse.fromClientResponse(clientResponse);
87+
assertNotNull(resp);
88+
assertEquals(201, resp.getHttpStatusCode());
89+
assertEquals(0, resp.getStatusCode());
90+
assertEquals("done", resp.getMsgDesc());
9391
}
9492

9593
@Test

‎agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,6 @@
5151
import static org.mockito.Mockito.mock;
5252
import static org.mockito.Mockito.when;
5353

54-
/**
55-
* @generated by Cursor
56-
* @description <Unit Test for RangerServiceDefHelper class>
57-
*/
5854
@ExtendWith(MockitoExtension.class)
5955
@TestMethodOrder(MethodOrderer.MethodName.class)
6056
public class TestRangerServiceDefHelper {

‎agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestRangerRESTClientDeadlock.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ public void testInterruptWorksWithMultipleURLs() throws Exception {
160160
HttpServer httpServer2 = HttpServer.create(new InetSocketAddress(0), 0);
161161
httpServer2.createContext("/", exchange -> {
162162
LOG.info("Server2: Received request, returning 503...");
163-
serverReceivedRequest.countDown(); // Count down the latch for server2 as well
164163
exchange.sendResponseHeaders(503, -1);
165164
exchange.close();
166165
});

‎plugin-nestedstructure/src/main/java/org/apache/ranger/authorization/nestedstructure/authorizer/RecordFilterJavaScript.java‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,26 @@ public class RecordFilterJavaScript {
5454

5555
private static final AtomicBoolean LOGGED_HOST_ACCESS_WARNING = new AtomicBoolean();
5656

57+
/**
58+
* Supported JavaScript engine names, in order of preference.
59+
* Graal.js is preferred for its security model and ECMAScript compliance.
60+
* Nashorn is available as a fallback on older JVMs.
61+
* js and JavaScript are generic names that may map to available engines.
62+
*/
63+
private static final String ENGINE_GRAAL_JS = "graal.js";
64+
private static final String ENGINE_NASHORN = "nashorn";
65+
private static final String ENGINE_JS = "js";
66+
private static final String ENGINE_JAVASCRIPT = "JavaScript";
67+
private static final String ENGINE_JAVASCRIPT_LOWERCASE = "javascript";
68+
69+
private static final String[] SUPPORTED_ENGINES = {
70+
ENGINE_GRAAL_JS,
71+
ENGINE_NASHORN,
72+
ENGINE_JS,
73+
ENGINE_JAVASCRIPT,
74+
ENGINE_JAVASCRIPT_LOWERCASE
75+
};
76+
5777
private RecordFilterJavaScript() {
5878
}
5979

@@ -146,10 +166,8 @@ public static boolean filterRow(String user, String filterExpr, String jsonStrin
146166
}
147167

148168
private static ScriptEngine resolveJavaScriptEngine(ScriptEngineManager mgr) {
149-
String[] tryNames = {"graal.js", "nashorn", "js", "JavaScript", "javascript"};
150-
151-
for (String name : tryNames) {
152-
ScriptEngine engine = mgr.getEngineByName(name);
169+
for (String engineName : SUPPORTED_ENGINES) {
170+
ScriptEngine engine = mgr.getEngineByName(engineName);
153171

154172
if (engine != null) {
155173
return engine;
@@ -165,7 +183,7 @@ private static boolean isGraalJsEngine(ScriptEngine engine) {
165183
}
166184

167185
try {
168-
return "graal.js".equalsIgnoreCase(engine.getFactory().getEngineName());
186+
return ENGINE_GRAAL_JS.equalsIgnoreCase(engine.getFactory().getEngineName());
169187
} catch (Exception e) {
170188
return false;
171189
}

0 commit comments

Comments
 (0)