rmuir commented on a change in pull request #1141: SOLR-14147 change the
Security manager to default to true.
URL: https://github.com/apache/lucene-solr/pull/1141#discussion_r373784595
##########
File path: solr/bin/solr
##########
@@ -2086,14 +2086,14 @@ else
REMOTE_JMX_OPTS=()
fi
-# Enable java security manager (limiting filesystem access and other things)
-if [ "$SOLR_SECURITY_MANAGER_ENABLED" == "true" ]; then
+# Disable java security manager (allowing filesystem access and other things)
+if [ "${SOLR_SECURITY_MANAGER_ENABLED:-true}" == "false" ]; then
Review comment:
as a matter of style/consistency i would reverse this so we are comparing
with "true", and change the comment back to "Enable" from "Disable". I think it
is easier on the brain to think about positives (not the negatives).
so something like this pseudocode:
```
# Enable java security manager (limiting filesystem access and other things)
if [ "$SOLR_SECURITY_MANAGER_ENABLED:-true" == "true" ]; then
SECURITY_MANAGER_OPTS=('-Djava.security.manager' \
"-Djava.security.policy=${SOLR_SERVER_DIR}/etc/security.policy" \
"-Djava.security.properties=${SOLR_SERVER_DIR}/etc/security.properties" \
'-Dsolr.internal.network.permission=*')
else
SECURITY_MANAGER_OPTS=()
fi
```
Essentially the same as existing logic, we just add the `:-true` so that if
the variable is not set by the user in any way, we default its value to true.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
