You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by GitBox <gi...@apache.org> on 2021/03/22 11:19:57 UTC
[GitHub] [unomi] jkevan opened a new pull request #266: Secure the unomi logs
jkevan opened a new pull request #266:
URL: https://github.com/apache/unomi/pull/266
--
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:
users@infra.apache.org
[GitHub] [unomi] jkevan merged pull request #266: Secure the unomi logs
Posted by GitBox <gi...@apache.org>.
jkevan merged pull request #266:
URL: https://github.com/apache/unomi/pull/266
--
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:
users@infra.apache.org
[GitHub] [unomi] sergehuber commented on a change in pull request #266: Secure the unomi logs
Posted by GitBox <gi...@apache.org>.
sergehuber commented on a change in pull request #266:
URL: https://github.com/apache/unomi/pull/266#discussion_r598682314
##########
File path: plugins/request/src/main/java/org/apache/unomi/plugins/request/actions/SetRemoteHostInfoAction.java
##########
@@ -232,9 +232,9 @@ private boolean ipLookupInDatabase(String remoteAddr, Session session) {
}
return true;
} catch (IOException | GeoIp2Exception e) {
- logger.warn("Cannot resolve IP {}, enable debug log level for complete stacktrace", remoteAddr);
+ logger.warn("Cannot resolve IP, enable debug log level for complete stacktrace");
if (logger.isDebugEnabled()) {
- logger.debug("Cannot resolve IP", e);
+ logger.debug("Cannot resolve IP: " + remoteAddr, e);
Review comment:
I don't always remember this, can't we use something like:
logger.debug("Cannot resolve IP: {} because of {}", remoteAddr, e)
--
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:
users@infra.apache.org
[GitHub] [unomi] jkevan commented on a change in pull request #266: Secure the unomi logs
Posted by GitBox <gi...@apache.org>.
jkevan commented on a change in pull request #266:
URL: https://github.com/apache/unomi/pull/266#discussion_r598910104
##########
File path: plugins/request/src/main/java/org/apache/unomi/plugins/request/actions/SetRemoteHostInfoAction.java
##########
@@ -232,9 +232,9 @@ private boolean ipLookupInDatabase(String remoteAddr, Session session) {
}
return true;
} catch (IOException | GeoIp2Exception e) {
- logger.warn("Cannot resolve IP {}, enable debug log level for complete stacktrace", remoteAddr);
+ logger.warn("Cannot resolve IP, enable debug log level for complete stacktrace");
if (logger.isDebugEnabled()) {
- logger.debug("Cannot resolve IP", e);
+ logger.debug("Cannot resolve IP: " + remoteAddr, e);
Review comment:
Yes I checked, we can do like this.
if the last parameter is an exception it will be interpreted as such regardless of the underlying framework.
--
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:
users@infra.apache.org
[GitHub] [unomi] sergehuber commented on a change in pull request #266: Secure the unomi logs
Posted by GitBox <gi...@apache.org>.
sergehuber commented on a change in pull request #266:
URL: https://github.com/apache/unomi/pull/266#discussion_r598992533
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java
##########
@@ -418,7 +438,10 @@ private Date getDate(Object value) {
try {
return Date.from(parser.parse(value.toString(), System::currentTimeMillis));
} catch (ElasticsearchParseException e) {
- logger.warn("unable to parse date " + value.toString(), e);
+ logger.warn("unable to parse date. See debug log level for full stacktrace");
+ if (logger.isDebugEnabled()) {
+ logger.debug("unable to parse date " + value.toString(), e);
Review comment:
Can we replace with parameterized message ?
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java
##########
@@ -185,7 +185,10 @@ public boolean eval(Condition condition, Item item, Map<String, Object> context,
if (!(e instanceof OgnlException)
|| (!StringUtils.startsWith(e.getMessage(),
"source is null for getProperty(null"))) {
- logger.warn("Error evaluating value for " + item.getClass().getName() + " " + name, e);
+ logger.warn("Error evaluating value for " + item.getClass().getName() + " " + name + ". See debug level for more information");
Review comment:
Can we replace with parameterized message ?
##########
File path: rest/src/main/java/org/apache/unomi/rest/ClusterServiceEndPoint.java
##########
@@ -91,7 +91,10 @@ public void purge(@PathParam("date") String date) {
try {
clusterService.purge(new SimpleDateFormat("yyyy-MM-dd").parse(date));
} catch (ParseException e) {
- logger.error("Cannot purge",e);
+ logger.error("Cannot parse date, expected format is: yyyy-MM-dd. See debug log level for more information");
+ if (logger.isDebugEnabled()) {
+ logger.debug("Cannot parse date " + date, e);
Review comment:
Can we replace with parameterized message ?
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java
##########
@@ -185,7 +185,10 @@ public boolean eval(Condition condition, Item item, Map<String, Object> context,
if (!(e instanceof OgnlException)
|| (!StringUtils.startsWith(e.getMessage(),
"source is null for getProperty(null"))) {
- logger.warn("Error evaluating value for " + item.getClass().getName() + " " + name, e);
+ logger.warn("Error evaluating value for " + item.getClass().getName() + " " + name + ". See debug level for more information");
+ if (logger.isDebugEnabled()) {
+ logger.debug("Error evaluating value for " + item.getClass().getName() + " " + name, e);
Review comment:
Can we replace with parameterized message ?
##########
File path: plugins/request/src/main/java/org/apache/unomi/plugins/request/actions/SetRemoteHostInfoAction.java
##########
@@ -246,9 +246,9 @@ private static boolean isAValidIPAddress(String remoteAddr) {
try {
addr = InetAddress.getByName(remoteAddr);
} catch (UnknownHostException e) {
- logger.warn("Cannot resolve IP {}, enable debug log level for complete stacktrace", remoteAddr);
+ logger.warn("Cannot resolve IP, enable debug log level for complete stacktrace");
if (logger.isDebugEnabled()) {
- logger.debug("Cannot resolve IP", e);
+ logger.debug("Cannot resolve IP: " + remoteAddr, e);
Review comment:
Can we replace with parameterized message ?
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetPropertyAction.java
##########
@@ -93,7 +93,10 @@ public int execute(Action action, Event event) {
date = event.getTimeStamp();
}
} catch (ParseException e) {
- logger.error("Error to parse date", e);
+ logger.error("Error parsing firstVisit date property. See debug log level for more information");
+ if (logger.isDebugEnabled()) {
+ logger.debug("Error parsing date: " + propertyFirstVisit, e);
Review comment:
Can we replace with parameterized message ?
--
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:
users@infra.apache.org