saxenapranav commented on code in PR #5447:
URL: https://github.com/apache/hadoop/pull/5447#discussion_r1127353729
##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/URLConnectionFactory.java:
##########
@@ -120,7 +136,7 @@ public HttpURLConnection configure(HttpURLConnection
connection)
}
}
- return conn;
+ return new BasicAuthConfigurator(conn, basicAuthCredentials);
Review Comment:
should we have something like:
```
private ConfigurationConfigurator getConfigurator(ConfigurationConfigurator
configurator, String basicAuthCred) {
if(basicAuthCred != null && !basicAuthCred.isEmpty()) {
return new BasicAuthConfigurator(configurator, basicAuthCred);}
else {return configurator;}
}
```
and instead of creating new object of basicAuthConfigurator, we call this
new method. And if an object of BasicAuthConfigurator can be made, method will
return that new object.
What you say?
##########
hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestBasicAuthConfigurator.java:
##########
@@ -0,0 +1,67 @@
+/**
+ * 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.hadoop.hdfs.web;
+
+import org.apache.hadoop.security.authentication.client.ConnectionConfigurator;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+
+public class TestBasicAuthConfigurator {
+ @Test
+ public void testNullCredentials() throws IOException {
+ ConnectionConfigurator conf = new BasicAuthConfigurator(null, null);
+ HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+ conf.configure(conn);
+ Mockito.verify(conn, Mockito.never()).setRequestProperty(Mockito.any(),
Mockito.any());
+ }
+
+ @Test
+ public void testEmptyCredentials() throws IOException {
+ ConnectionConfigurator conf = new BasicAuthConfigurator(null, "");
+ HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+ conf.configure(conn);
+ Mockito.verify(conn, Mockito.never()).setRequestProperty(Mockito.any(),
Mockito.any());
+ }
+
+ @Test
+ public void testCredentialsSet() throws IOException {
+ ConnectionConfigurator conf = new BasicAuthConfigurator(null, "user:pass");
+ HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+ conf.configure(conn);
+ Mockito.verify(conn, Mockito.times(1)).setRequestProperty(
+ "AUTHORIZATION",
+ "Basic dXNlcjpwYXNz"
+ );
+ }
+
+ @Test
+ public void testParentConfigurator() throws IOException {
+ ConnectionConfigurator parent = Mockito.mock(ConnectionConfigurator.class);
+ ConnectionConfigurator conf = new BasicAuthConfigurator(parent,
"user:pass");
+ HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+ conf.configure(conn);
+ Mockito.verify(conn, Mockito.times(1)).setRequestProperty(
+ "AUTHORIZATION",
+ "Basic dXNlcjpwYXNz"
Review Comment:
should we have` "Basic " + Base64.getEncoder().encodeToString(
credentials.getBytes(StandardCharsets.UTF_8)`
as its not clear from test how user:pass is converting to `dXNlcjpwYXNz`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
