[ 
https://issues.apache.org/jira/browse/HADOOP-18496?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17764776#comment-17764776
 ] 

ASF GitHub Bot commented on HADOOP-18496:
-----------------------------------------

steveloughran commented on code in PR #6057:
URL: https://github.com/apache/hadoop/pull/6057#discussion_r1324778866


##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/oauth2/CredentialBasedAccessTokenProvider.java:
##########
@@ -97,38 +104,37 @@ public synchronized String getAccessToken() throws 
IOException {
   }
 
   void refresh() throws IOException {
-    OkHttpClient client = new OkHttpClient.Builder()
-            .connectTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT, 
TimeUnit.MILLISECONDS)
-            .readTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT, 
TimeUnit.MILLISECONDS)
-            .build();
-
-    String bodyString = Utils.postBody(CLIENT_SECRET, getCredential(),
-        GRANT_TYPE, CLIENT_CREDENTIALS,
-        CLIENT_ID, clientId);
-
-    RequestBody body = RequestBody.create(bodyString, URLENCODED);
-
-    Request request = new Request.Builder()
-        .url(refreshURL)
-        .post(body)
+    final List<NameValuePair> pairs = new ArrayList<>();
+    pairs.add(new BasicNameValuePair(CLIENT_SECRET, getCredential()));
+    pairs.add(new BasicNameValuePair(GRANT_TYPE, CLIENT_CREDENTIALS));
+    pairs.add(new BasicNameValuePair(CLIENT_ID, clientId));
+    final RequestConfig config = RequestConfig.custom()
+        .setConnectTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
+        
.setConnectionRequestTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
+        .setSocketTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
         .build();
-    try (Response response = client.newCall(request).execute()) {
-      if (!response.isSuccessful()) {
-        throw new IOException("Unexpected code " + response);
-      }
-
-      if (response.code() != HttpStatus.SC_OK) {
-        throw new IllegalArgumentException("Received invalid http response: "
-            + response.code() + ", text = " + response.toString());
+    try (CloseableHttpClient client =
+             
HttpClientBuilder.create().setDefaultRequestConfig(config).build()) {
+      final HttpPost httpPost = new HttpPost(refreshURL);
+      httpPost.setEntity(new UrlEncodedFormEntity(pairs, 
StandardCharsets.UTF_8));
+      httpPost.setHeader(HttpHeaders.CONTENT_TYPE, URLENCODED);
+      try (CloseableHttpResponse response = client.execute(httpPost)) {
+        final int statusCode = response.getStatusLine().getStatusCode();
+        if (statusCode != HttpStatus.SC_OK) {
+          throw new IllegalArgumentException(
+              "Received invalid http response: " + statusCode + ", text = " +
+                  EntityUtils.toString(response.getEntity()));
+        }
+        Map<?, ?> responseBody = JsonSerialization.mapReader().readValue(

Review Comment:
   let's check the return content type too; been burned by both proxies and how 
the abfs oauth failure is 200 + text/html for users.



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/oauth2/CredentialBasedAccessTokenProvider.java:
##########
@@ -97,38 +104,37 @@ public synchronized String getAccessToken() throws 
IOException {
   }
 
   void refresh() throws IOException {
-    OkHttpClient client = new OkHttpClient.Builder()
-            .connectTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT, 
TimeUnit.MILLISECONDS)
-            .readTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT, 
TimeUnit.MILLISECONDS)
-            .build();
-
-    String bodyString = Utils.postBody(CLIENT_SECRET, getCredential(),
-        GRANT_TYPE, CLIENT_CREDENTIALS,
-        CLIENT_ID, clientId);
-
-    RequestBody body = RequestBody.create(bodyString, URLENCODED);
-
-    Request request = new Request.Builder()
-        .url(refreshURL)
-        .post(body)
+    final List<NameValuePair> pairs = new ArrayList<>();
+    pairs.add(new BasicNameValuePair(CLIENT_SECRET, getCredential()));
+    pairs.add(new BasicNameValuePair(GRANT_TYPE, CLIENT_CREDENTIALS));
+    pairs.add(new BasicNameValuePair(CLIENT_ID, clientId));
+    final RequestConfig config = RequestConfig.custom()
+        .setConnectTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
+        
.setConnectionRequestTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
+        .setSocketTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
         .build();
-    try (Response response = client.newCall(request).execute()) {
-      if (!response.isSuccessful()) {
-        throw new IOException("Unexpected code " + response);
-      }
-
-      if (response.code() != HttpStatus.SC_OK) {
-        throw new IllegalArgumentException("Received invalid http response: "
-            + response.code() + ", text = " + response.toString());
+    try (CloseableHttpClient client =
+             
HttpClientBuilder.create().setDefaultRequestConfig(config).build()) {
+      final HttpPost httpPost = new HttpPost(refreshURL);
+      httpPost.setEntity(new UrlEncodedFormEntity(pairs, 
StandardCharsets.UTF_8));
+      httpPost.setHeader(HttpHeaders.CONTENT_TYPE, URLENCODED);
+      try (CloseableHttpResponse response = client.execute(httpPost)) {
+        final int statusCode = response.getStatusLine().getStatusCode();
+        if (statusCode != HttpStatus.SC_OK) {
+          throw new IllegalArgumentException(
+              "Received invalid http response: " + statusCode + ", text = " +
+                  EntityUtils.toString(response.getEntity()));
+        }
+        Map<?, ?> responseBody = JsonSerialization.mapReader().readValue(
+            EntityUtils.toString(response.getEntity()));
+
+        String newExpiresIn = responseBody.get(EXPIRES_IN).toString();
+        timer.setExpiresIn(newExpiresIn);
+
+        accessToken = responseBody.get(ACCESS_TOKEN).toString();
       }
-
-      Map<?, ?> responseBody = JsonSerialization.mapReader().readValue(
-          response.body().string());
-
-      String newExpiresIn = responseBody.get(EXPIRES_IN).toString();
-      timer.setExpiresIn(newExpiresIn);
-
-      accessToken = responseBody.get(ACCESS_TOKEN).toString();
+    } catch (RuntimeException e) {
+      throw new IOException("Unable to obtain access token from credential", 
e);
     } catch (Exception e) {

Review Comment:
   this is now a duplicate of the handler above, isn't it? so they can be 
collapsed



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/oauth2/CredentialBasedAccessTokenProvider.java:
##########
@@ -97,38 +104,37 @@ public synchronized String getAccessToken() throws 
IOException {
   }
 
   void refresh() throws IOException {
-    OkHttpClient client = new OkHttpClient.Builder()
-            .connectTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT, 
TimeUnit.MILLISECONDS)
-            .readTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT, 
TimeUnit.MILLISECONDS)
-            .build();
-
-    String bodyString = Utils.postBody(CLIENT_SECRET, getCredential(),
-        GRANT_TYPE, CLIENT_CREDENTIALS,
-        CLIENT_ID, clientId);
-
-    RequestBody body = RequestBody.create(bodyString, URLENCODED);
-
-    Request request = new Request.Builder()
-        .url(refreshURL)
-        .post(body)
+    final List<NameValuePair> pairs = new ArrayList<>();
+    pairs.add(new BasicNameValuePair(CLIENT_SECRET, getCredential()));
+    pairs.add(new BasicNameValuePair(GRANT_TYPE, CLIENT_CREDENTIALS));
+    pairs.add(new BasicNameValuePair(CLIENT_ID, clientId));
+    final RequestConfig config = RequestConfig.custom()
+        .setConnectTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
+        
.setConnectionRequestTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
+        .setSocketTimeout(URLConnectionFactory.DEFAULT_SOCKET_TIMEOUT)
         .build();
-    try (Response response = client.newCall(request).execute()) {
-      if (!response.isSuccessful()) {
-        throw new IOException("Unexpected code " + response);
-      }
-
-      if (response.code() != HttpStatus.SC_OK) {
-        throw new IllegalArgumentException("Received invalid http response: "
-            + response.code() + ", text = " + response.toString());
+    try (CloseableHttpClient client =
+             
HttpClientBuilder.create().setDefaultRequestConfig(config).build()) {
+      final HttpPost httpPost = new HttpPost(refreshURL);
+      httpPost.setEntity(new UrlEncodedFormEntity(pairs, 
StandardCharsets.UTF_8));
+      httpPost.setHeader(HttpHeaders.CONTENT_TYPE, URLENCODED);
+      try (CloseableHttpResponse response = client.execute(httpPost)) {
+        final int statusCode = response.getStatusLine().getStatusCode();
+        if (statusCode != HttpStatus.SC_OK) {
+          throw new IllegalArgumentException(
+              "Received invalid http response: " + statusCode + ", text = " +
+                  EntityUtils.toString(response.getEntity()));
+        }
+        Map<?, ?> responseBody = JsonSerialization.mapReader().readValue(
+            EntityUtils.toString(response.getEntity()));
+
+        String newExpiresIn = responseBody.get(EXPIRES_IN).toString();
+        timer.setExpiresIn(newExpiresIn);
+
+        accessToken = responseBody.get(ACCESS_TOKEN).toString();
       }
-
-      Map<?, ?> responseBody = JsonSerialization.mapReader().readValue(
-          response.body().string());
-
-      String newExpiresIn = responseBody.get(EXPIRES_IN).toString();
-      timer.setExpiresIn(newExpiresIn);
-
-      accessToken = responseBody.get(ACCESS_TOKEN).toString();
+    } catch (RuntimeException e) {
+      throw new IOException("Unable to obtain access token from credential", 
e);

Review Comment:
   add e.toString to the text





> upgrade kotlin-stdlib due to CVEs
> ---------------------------------
>
>                 Key: HADOOP-18496
>                 URL: https://issues.apache.org/jira/browse/HADOOP-18496
>             Project: Hadoop Common
>          Issue Type: Improvement
>            Reporter: PJ Fanning
>            Priority: Major
>              Labels: pull-request-available
>
> I'm not an expert on Kotlin but dependabot show these 2 CVEs with the version 
> of kotlin-stdlib used in Hadoop.
>  * [https://github.com/advisories/GHSA-cqj8-47ch-rvvq]
>  * [https://github.com/advisories/GHSA-2qp4-g3q3-f92w]
> kotlin-stlib 1.6.0 is the minimum version needed to fix both. It might be 
> better to use latest v1.6 jar (currently 1.6.21) or even use latest jar 
> altogether (currently 1.7.20).
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to