cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654980982



##########
File path: java/org/apache/catalina/realm/GenericPrincipal.java
##########
@@ -304,10 +333,165 @@ public void logout() throws Exception {
     }
 
 
+    @Override
+    public Object getAttribute(String name) {
+        if (attributes == null) {
+            return null;
+        }
+        Object value = attributes.get(name);
+        if (value == null) {
+            return null;
+        }
+        Object copy = copyObject(value);
+        return copy != null ? copy : value.toString();
+    }
+
+
+    @Override
+    public Enumeration<String> getAttributeNames() {
+        if (attributes == null) {
+            return Collections.emptyEnumeration();
+        }
+        return Collections.enumeration(attributes.keySet());
+    }
+
+
+    @Override
+    public boolean isAttributesCaseIgnored() {
+        return (attributes instanceof CaseInsensitiveKeyMap<?>);
+    }
+
+
+    /**
+     * Creates and returns a deep copy of the specified object. Deep-copying
+     * works only for objects of a couple of <i>hard coded</i> types or, if the
+     * object implements <code>java.io.Serializable</code>. In all other cases,
+     * this method returns <code>null</code>.
+     * 
+     * @param obj the object to copy
+     * 
+     * @return a deep copied clone of the specified object, or 
<code>null</code>
+     *         if deep-copying was not possible
+     */
+    private Object copyObject(Object obj) {
+
+        // first, try some commonly used object types
+        if (obj instanceof String) {
+            return new String((String) obj);
+
+        } else if (obj instanceof Integer) {
+            return Integer.valueOf((int) obj);
+
+        } else if (obj instanceof Long) {
+            return Long.valueOf((long) obj);
+
+        } else if (obj instanceof Boolean) {
+            return Boolean.valueOf((boolean) obj);
+
+        } else if (obj instanceof Double) {
+            return Double.valueOf((double) obj);
+
+        } else if (obj instanceof Float) {
+            return Float.valueOf((float) obj);
+
+        } else if (obj instanceof Character) {
+            return Character.valueOf((char) obj);
+
+        } else if (obj instanceof Byte) {
+            return Byte.valueOf((byte) obj); 
+
+        } else if (obj instanceof Short) {
+            return Short.valueOf((short) obj);
+
+        } else if (obj instanceof BigDecimal) {
+            return new BigDecimal(((BigDecimal) obj).toString());
+
+        } else if (obj instanceof BigInteger) {
+            return new BigInteger(((BigInteger) obj).toString());
+
+        }
+
+        // Date and JDBC date and time types
+        else if (obj instanceof java.sql.Date) {
+            return ((java.sql.Date) obj).clone();
+
+        } else if (obj instanceof java.sql.Time) {
+            return ((java.sql.Time) obj).clone();
+
+        } else if (obj instanceof java.sql.Timestamp) {
+            return ((java.sql.Timestamp) obj).clone();
+
+        } else if (obj instanceof Date) {
+            return ((Date) obj).clone();
+
+        }
+
+        // these types may come up as well
+        else if (obj instanceof URI) {
+            try {
+                return new URI(((URI) obj).toString());
+            } catch (URISyntaxException e) {
+                // not expected
+            }
+        } else if (obj instanceof URL) {
+            try {
+                return new URI(((URL) obj).toString());
+            } catch (URISyntaxException e) {
+                // not expected
+            }
+        } else if (obj instanceof UUID) {
+            return new UUID(((UUID) obj).getMostSignificantBits(),
+                    ((UUID) obj).getLeastSignificantBits());
+
+        }

Review comment:
       I was discussing that with Mark and Chris. Both are sure that we need to 
go with _defensive copies_. Chris was suggesting, that `Cloneable` is not the 
best choice. Also, with reflection (and also with JNI), it is still possible to 
change the value of a final field, so there are no really immutable objects.




-- 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to