You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2022/12/25 21:38:17 UTC
[logging-log4j2] branch master updated: Fix NPE in Log4jMarker
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/master by this push:
new ffc82a4820 Fix NPE in Log4jMarker
ffc82a4820 is described below
commit ffc82a48203fb890d85e3a497476611368ba1021
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Sun Dec 25 22:30:27 2022 +0100
Fix NPE in Log4jMarker
Log4j2's Marker#getParents() does actually return `null` instead of an
empty array.
---
.../java/org/apache/logging/slf4j/Log4jMarker.java | 83 ++++++++++------------
.../java/org/apache/logging/slf4j/Log4jMarker.java | 9 +--
2 files changed, 44 insertions(+), 48 deletions(-)
diff --git a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
index 30931851f0..bb1621aa39 100644
--- a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
+++ b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
@@ -17,8 +17,10 @@
package org.apache.logging.slf4j;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Iterator;
import java.util.List;
+import java.util.Objects;
import org.apache.logging.log4j.MarkerManager;
import org.slf4j.IMarkerFactory;
@@ -46,47 +48,40 @@ class Log4jMarker implements Marker {
@Override
public void add(final Marker marker) {
- if (marker == null) {
- throw new IllegalArgumentException();
- }
+ if (marker == null) {
+ throw new IllegalArgumentException();
+ }
final Marker m = factory.getMarker(marker.getName());
this.marker.addParents(((Log4jMarker)m).getLog4jMarker());
}
@Override
- public boolean contains(final Marker marker) {
- if (marker == null) {
- throw new IllegalArgumentException();
- }
- return this.marker.isInstanceOf(marker.getName());
- }
+ public boolean contains(final Marker marker) {
+ if (marker == null) {
+ throw new IllegalArgumentException();
+ }
+ return this.marker.isInstanceOf(marker.getName());
+ }
@Override
- public boolean contains(final String s) {
- return s != null && this.marker.isInstanceOf(s);
- }
+ public boolean contains(final String s) {
+ return s != null ? this.marker.isInstanceOf(s) : false;
+ }
@Override
- public boolean equals(final Object obj) {
- if (this == obj) {
- return true;
- }
- if (obj == null) {
- return false;
- }
- if (!(obj instanceof Log4jMarker)) {
- return false;
- }
- final Log4jMarker other = (Log4jMarker) obj;
- if (marker == null) {
- if (other.marker != null) {
- return false;
- }
- } else if (!marker.equals(other.marker)) {
- return false;
- }
- return true;
- }
+ public boolean equals(final Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null) {
+ return false;
+ }
+ if (!(obj instanceof Log4jMarker)) {
+ return false;
+ }
+ final Log4jMarker other = (Log4jMarker) obj;
+ return Objects.equals(marker, other.marker);
+ }
public org.apache.logging.log4j.Marker getLog4jMarker() {
return marker;
@@ -103,30 +98,30 @@ class Log4jMarker implements Marker {
}
@Override
- public int hashCode() {
- final int prime = 31;
- int result = 1;
- result = prime * result + ((marker == null) ? 0 : marker.hashCode());
- return result;
- }
+ public int hashCode() {
+ return 31 + Objects.hashCode(marker);
+ }
@Override
public boolean hasReferences() {
return marker.hasParents();
}
- @Override
+ @Override
public Iterator<Marker> iterator() {
final org.apache.logging.log4j.Marker[] log4jParents = this.marker.getParents();
+ if (log4jParents == null) {
+ return Collections.emptyIterator();
+ }
final List<Marker> parents = new ArrayList<>(log4jParents.length);
- for (final org.apache.logging.log4j.Marker m : log4jParents) {
+ for (final org.apache.logging.log4j.Marker m : log4jParents) {
parents.add(factory.getMarker(m.getName()));
}
return parents.iterator();
}
- @Override
- public boolean remove(final Marker marker) {
- return marker != null && this.marker.remove(MarkerManager.getMarker(marker.getName()));
- }
+ @Override
+ public boolean remove(final Marker marker) {
+ return marker != null ? this.marker.remove(MarkerManager.getMarker(marker.getName())) : false;
+ }
}
diff --git a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
index 1b96e1a010..bb1621aa39 100644
--- a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
+++ b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
@@ -17,6 +17,7 @@
package org.apache.logging.slf4j;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
@@ -79,10 +80,7 @@ class Log4jMarker implements Marker {
return false;
}
final Log4jMarker other = (Log4jMarker) obj;
- if (!Objects.equals(marker, other.marker)) {
- return false;
- }
- return true;
+ return Objects.equals(marker, other.marker);
}
public org.apache.logging.log4j.Marker getLog4jMarker() {
@@ -112,6 +110,9 @@ class Log4jMarker implements Marker {
@Override
public Iterator<Marker> iterator() {
final org.apache.logging.log4j.Marker[] log4jParents = this.marker.getParents();
+ if (log4jParents == null) {
+ return Collections.emptyIterator();
+ }
final List<Marker> parents = new ArrayList<>(log4jParents.length);
for (final org.apache.logging.log4j.Marker m : log4jParents) {
parents.add(factory.getMarker(m.getName()));
Re: [logging-log4j2] branch master updated: Fix NPE in Log4jMarker
Posted by Gary Gregory <ga...@gmail.com>.
Thank you for the clarification, Piotr. I missed the fact that this is
for slf4j.
Gary
On Sun, Dec 25, 2022 at 5:03 PM Piotr P. Karwasz
<pi...@gmail.com> wrote:
>
> Hi Gary,
>
> On Sun, 25 Dec 2022 at 22:53, Gary Gregory <ga...@gmail.com> wrote:
> >
> > I think using Objects.requireNonNull() is more precise FWIW.
>
> I agree with you, but Ceki's javadoc requires an IllegalArgumentException.
>
> Piotr
Re: [logging-log4j2] branch master updated: Fix NPE in Log4jMarker
Posted by "Piotr P. Karwasz" <pi...@gmail.com>.
Hi Gary,
On Sun, 25 Dec 2022 at 22:53, Gary Gregory <ga...@gmail.com> wrote:
>
> I think using Objects.requireNonNull() is more precise FWIW.
I agree with you, but Ceki's javadoc requires an IllegalArgumentException.
Piotr
Re: [logging-log4j2] branch master updated: Fix NPE in Log4jMarker
Posted by Gary Gregory <ga...@gmail.com>.
I think using Objects.requireNonNull() is more precise FWIW.
Gary
On Sun, Dec 25, 2022, 16:38 <pk...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> pkarwasz pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new ffc82a4820 Fix NPE in Log4jMarker
> ffc82a4820 is described below
>
> commit ffc82a48203fb890d85e3a497476611368ba1021
> Author: Piotr P. Karwasz <pi...@karwasz.org>
> AuthorDate: Sun Dec 25 22:30:27 2022 +0100
>
> Fix NPE in Log4jMarker
>
> Log4j2's Marker#getParents() does actually return `null` instead of an
> empty array.
> ---
> .../java/org/apache/logging/slf4j/Log4jMarker.java | 83
> ++++++++++------------
> .../java/org/apache/logging/slf4j/Log4jMarker.java | 9 +--
> 2 files changed, 44 insertions(+), 48 deletions(-)
>
> diff --git
> a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
> b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
> index 30931851f0..bb1621aa39 100644
> ---
> a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
> +++
> b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
> @@ -17,8 +17,10 @@
> package org.apache.logging.slf4j;
>
> import java.util.ArrayList;
> +import java.util.Collections;
> import java.util.Iterator;
> import java.util.List;
> +import java.util.Objects;
>
> import org.apache.logging.log4j.MarkerManager;
> import org.slf4j.IMarkerFactory;
> @@ -46,47 +48,40 @@ class Log4jMarker implements Marker {
>
> @Override
> public void add(final Marker marker) {
> - if (marker == null) {
> - throw new IllegalArgumentException();
> - }
> + if (marker == null) {
> + throw new IllegalArgumentException();
> + }
> final Marker m = factory.getMarker(marker.getName());
> this.marker.addParents(((Log4jMarker)m).getLog4jMarker());
> }
>
> @Override
> - public boolean contains(final Marker marker) {
> - if (marker == null) {
> - throw new IllegalArgumentException();
> - }
> - return this.marker.isInstanceOf(marker.getName());
> - }
> + public boolean contains(final Marker marker) {
> + if (marker == null) {
> + throw new IllegalArgumentException();
> + }
> + return this.marker.isInstanceOf(marker.getName());
> + }
>
> @Override
> - public boolean contains(final String s) {
> - return s != null && this.marker.isInstanceOf(s);
> - }
> + public boolean contains(final String s) {
> + return s != null ? this.marker.isInstanceOf(s) : false;
> + }
>
> @Override
> - public boolean equals(final Object obj) {
> - if (this == obj) {
> - return true;
> - }
> - if (obj == null) {
> - return false;
> - }
> - if (!(obj instanceof Log4jMarker)) {
> - return false;
> - }
> - final Log4jMarker other = (Log4jMarker) obj;
> - if (marker == null) {
> - if (other.marker != null) {
> - return false;
> - }
> - } else if (!marker.equals(other.marker)) {
> - return false;
> - }
> - return true;
> - }
> + public boolean equals(final Object obj) {
> + if (this == obj) {
> + return true;
> + }
> + if (obj == null) {
> + return false;
> + }
> + if (!(obj instanceof Log4jMarker)) {
> + return false;
> + }
> + final Log4jMarker other = (Log4jMarker) obj;
> + return Objects.equals(marker, other.marker);
> + }
>
> public org.apache.logging.log4j.Marker getLog4jMarker() {
> return marker;
> @@ -103,30 +98,30 @@ class Log4jMarker implements Marker {
> }
>
> @Override
> - public int hashCode() {
> - final int prime = 31;
> - int result = 1;
> - result = prime * result + ((marker == null) ? 0 :
> marker.hashCode());
> - return result;
> - }
> + public int hashCode() {
> + return 31 + Objects.hashCode(marker);
> + }
>
> @Override
> public boolean hasReferences() {
> return marker.hasParents();
> }
>
> - @Override
> + @Override
> public Iterator<Marker> iterator() {
> final org.apache.logging.log4j.Marker[] log4jParents =
> this.marker.getParents();
> + if (log4jParents == null) {
> + return Collections.emptyIterator();
> + }
> final List<Marker> parents = new ArrayList<>(log4jParents.length);
> - for (final org.apache.logging.log4j.Marker m :
> log4jParents) {
> + for (final org.apache.logging.log4j.Marker m : log4jParents) {
> parents.add(factory.getMarker(m.getName()));
> }
> return parents.iterator();
> }
>
> - @Override
> - public boolean remove(final Marker marker) {
> - return marker != null &&
> this.marker.remove(MarkerManager.getMarker(marker.getName()));
> - }
> + @Override
> + public boolean remove(final Marker marker) {
> + return marker != null ?
> this.marker.remove(MarkerManager.getMarker(marker.getName())) : false;
> + }
> }
> diff --git
> a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
> b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
> index 1b96e1a010..bb1621aa39 100644
> ---
> a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
> +++
> b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java
> @@ -17,6 +17,7 @@
> package org.apache.logging.slf4j;
>
> import java.util.ArrayList;
> +import java.util.Collections;
> import java.util.Iterator;
> import java.util.List;
> import java.util.Objects;
> @@ -79,10 +80,7 @@ class Log4jMarker implements Marker {
> return false;
> }
> final Log4jMarker other = (Log4jMarker) obj;
> - if (!Objects.equals(marker, other.marker)) {
> - return false;
> - }
> - return true;
> + return Objects.equals(marker, other.marker);
> }
>
> public org.apache.logging.log4j.Marker getLog4jMarker() {
> @@ -112,6 +110,9 @@ class Log4jMarker implements Marker {
> @Override
> public Iterator<Marker> iterator() {
> final org.apache.logging.log4j.Marker[] log4jParents =
> this.marker.getParents();
> + if (log4jParents == null) {
> + return Collections.emptyIterator();
> + }
> final List<Marker> parents = new ArrayList<>(log4jParents.length);
> for (final org.apache.logging.log4j.Marker m : log4jParents) {
> parents.add(factory.getMarker(m.getName()));
>
>