[ 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