Re: [commons-codec] branch master updated: Change AssertionError to IllegalStateException
You can recover from an exception, errors you are not supposed to...
Gary
On Sat, Dec 28, 2019, 13:53 Matt Sicker wrote:
> Aren’t Errors supposed to be fatal exceptions that are generally only
> caught by exception handlers rather than by calling code? Think
> OutOfMemoryError or StackOverflowError.
>
> On Sat, Dec 28, 2019 at 11:35 Miguel Munoz .invalid>
> wrote:
>
> > I can't look at the code right now, but I would use an
> > IllegalStateException if the problem is caused by a user error. I would
> use
> > an AssertionError if the problem is caused by a defect in the
> > implementation.
> >
> > -- Miguel Muñoz
> > On Friday, December 27, 2019, 5:45:58 PM PST, Gary Gregory <
> > garydgreg...@gmail.com> wrote:
> >
> > On Fri, Dec 27, 2019 at 8:17 PM wrote:
> >
> > > This is an automated email from the ASF dual-hosted git repository.
> > >
> > > aherbert pushed a commit to branch master
> > > in repository https://gitbox.apache.org/repos/asf/commons-codec.git
> > >
> > >
> > > The following commit(s) were added to refs/heads/master by this push:
> > > new 1cf4d19 Change AssertionError to IllegalStateException
> > > 1cf4d19 is described below
> > >
> > > commit 1cf4d19069c64d0493f8b92178ffdb728c0c0ac2
> > > Author: Alex Herbert
> > > AuthorDate: Sat Dec 28 01:17:17 2019 +
> > >
> > >Change AssertionError to IllegalStateException
> > > ---
> > > src/main/java/org/apache/commons/codec/digest/MurmurHash3.java | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git
> > > a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > > b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > > index d4a95ea..5d9aa9d 100644
> > > --- a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > > +++ b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > > @@ -1054,7 +1054,7 @@ public final class MurmurHash3 {
> > > k = orBytes(unprocessed[0], unprocessed[1],
> > > unprocessed[2], data[offset]);
> > > break;
> > > default:
> > > -throw new AssertionError("Unprocessed length
> should
> > > be 1, 2, or 3: " + unprocessedLength);
> > > +throw new IllegalStateException("Unprocessed
> length
> > > should be 1, 2, or 3: " + unprocessedLength);
> > > }
> > > hash = mix32(k, hash);
> > > // Update the offset and length
> > >
> >
> > That seems clearer, thanks.
> >
> > This seems like the kind of code we might want fuzz test. It seems
> > quite unlikely otherwise we'd hit this case. I also wonder if
> >
> > Thoughts?
> >
> > I see in several places:
> >
> > // Note: This fails to apply masking using 0xL to the seed.
> >
> > Shouldn't this be in the Javadoc?
> >
> > Gary
> >
>
> --
> Matt Sicker
>
Re: [commons-codec] branch master updated: Change AssertionError to IllegalStateException
Aren’t Errors supposed to be fatal exceptions that are generally only
caught by exception handlers rather than by calling code? Think
OutOfMemoryError or StackOverflowError.
On Sat, Dec 28, 2019 at 11:35 Miguel Munoz
wrote:
> I can't look at the code right now, but I would use an
> IllegalStateException if the problem is caused by a user error. I would use
> an AssertionError if the problem is caused by a defect in the
> implementation.
>
> -- Miguel Muñoz
> On Friday, December 27, 2019, 5:45:58 PM PST, Gary Gregory <
> garydgreg...@gmail.com> wrote:
>
> On Fri, Dec 27, 2019 at 8:17 PM wrote:
>
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > aherbert pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/commons-codec.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new 1cf4d19 Change AssertionError to IllegalStateException
> > 1cf4d19 is described below
> >
> > commit 1cf4d19069c64d0493f8b92178ffdb728c0c0ac2
> > Author: Alex Herbert
> > AuthorDate: Sat Dec 28 01:17:17 2019 +
> >
> >Change AssertionError to IllegalStateException
> > ---
> > src/main/java/org/apache/commons/codec/digest/MurmurHash3.java | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> > a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > index d4a95ea..5d9aa9d 100644
> > --- a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > +++ b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > @@ -1054,7 +1054,7 @@ public final class MurmurHash3 {
> > k = orBytes(unprocessed[0], unprocessed[1],
> > unprocessed[2], data[offset]);
> > break;
> > default:
> > -throw new AssertionError("Unprocessed length should
> > be 1, 2, or 3: " + unprocessedLength);
> > +throw new IllegalStateException("Unprocessed length
> > should be 1, 2, or 3: " + unprocessedLength);
> > }
> > hash = mix32(k, hash);
> > // Update the offset and length
> >
>
> That seems clearer, thanks.
>
> This seems like the kind of code we might want fuzz test. It seems
> quite unlikely otherwise we'd hit this case. I also wonder if
>
> Thoughts?
>
> I see in several places:
>
> // Note: This fails to apply masking using 0xL to the seed.
>
> Shouldn't this be in the Javadoc?
>
> Gary
>
--
Matt Sicker
Re: [commons-codec] branch master updated: Change AssertionError to IllegalStateException
I can't look at the code right now, but I would use an IllegalStateException
if the problem is caused by a user error. I would use an AssertionError if the
problem is caused by a defect in the implementation.
-- Miguel Muñoz
On Friday, December 27, 2019, 5:45:58 PM PST, Gary Gregory
wrote:
On Fri, Dec 27, 2019 at 8:17 PM wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> aherbert pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/commons-codec.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 1cf4d19 Change AssertionError to IllegalStateException
> 1cf4d19 is described below
>
> commit 1cf4d19069c64d0493f8b92178ffdb728c0c0ac2
> Author: Alex Herbert
> AuthorDate: Sat Dec 28 01:17:17 2019 +
>
> Change AssertionError to IllegalStateException
> ---
> src/main/java/org/apache/commons/codec/digest/MurmurHash3.java | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> index d4a95ea..5d9aa9d 100644
> --- a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> +++ b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> @@ -1054,7 +1054,7 @@ public final class MurmurHash3 {
> k = orBytes(unprocessed[0], unprocessed[1],
> unprocessed[2], data[offset]);
> break;
> default:
> - throw new AssertionError("Unprocessed length should
> be 1, 2, or 3: " + unprocessedLength);
> + throw new IllegalStateException("Unprocessed length
> should be 1, 2, or 3: " + unprocessedLength);
> }
> hash = mix32(k, hash);
> // Update the offset and length
>
That seems clearer, thanks.
This seems like the kind of code we might want fuzz test. It seems
quite unlikely otherwise we'd hit this case. I also wonder if
Thoughts?
I see in several places:
// Note: This fails to apply masking using 0xL to the seed.
Shouldn't this be in the Javadoc?
Gary
Re: [commons-codec] branch master updated: Change AssertionError to IllegalStateException
Wherever the note is found the javadoc should include
* This implementation contains a sign-extension bug in the seed
initialization.
* This manifests if the seed is negative.
On Sat, Dec 28, 2019 at 1:45 AM Gary Gregory wrote:
> On Fri, Dec 27, 2019 at 8:17 PM wrote:
>
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > aherbert pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/commons-codec.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new 1cf4d19 Change AssertionError to IllegalStateException
> > 1cf4d19 is described below
> >
> > commit 1cf4d19069c64d0493f8b92178ffdb728c0c0ac2
> > Author: Alex Herbert
> > AuthorDate: Sat Dec 28 01:17:17 2019 +
> >
> > Change AssertionError to IllegalStateException
> > ---
> > src/main/java/org/apache/commons/codec/digest/MurmurHash3.java | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> > a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > index d4a95ea..5d9aa9d 100644
> > --- a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > +++ b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> > @@ -1054,7 +1054,7 @@ public final class MurmurHash3 {
> > k = orBytes(unprocessed[0], unprocessed[1],
> > unprocessed[2], data[offset]);
> > break;
> > default:
> > -throw new AssertionError("Unprocessed length should
> > be 1, 2, or 3: " + unprocessedLength);
> > +throw new IllegalStateException("Unprocessed length
> > should be 1, 2, or 3: " + unprocessedLength);
> > }
> > hash = mix32(k, hash);
> > // Update the offset and length
> >
>
> That seems clearer, thanks.
>
> This seems like the kind of code we might want fuzz test. It seems
> quite unlikely otherwise we'd hit this case. I also wonder if
>
> Thoughts?
>
> I see in several places:
>
> // Note: This fails to apply masking using 0xL to the seed.
>
> Shouldn't this be in the Javadoc?
>
> Gary
>
--
I like: Like Like - The likeliest place on the web
LinkedIn: http://www.linkedin.com/in/claudewarren
Re: [commons-codec] branch master updated: Change AssertionError to IllegalStateException
On Fri, Dec 27, 2019 at 8:17 PM wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> aherbert pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/commons-codec.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 1cf4d19 Change AssertionError to IllegalStateException
> 1cf4d19 is described below
>
> commit 1cf4d19069c64d0493f8b92178ffdb728c0c0ac2
> Author: Alex Herbert
> AuthorDate: Sat Dec 28 01:17:17 2019 +
>
> Change AssertionError to IllegalStateException
> ---
> src/main/java/org/apache/commons/codec/digest/MurmurHash3.java | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> index d4a95ea..5d9aa9d 100644
> --- a/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> +++ b/src/main/java/org/apache/commons/codec/digest/MurmurHash3.java
> @@ -1054,7 +1054,7 @@ public final class MurmurHash3 {
> k = orBytes(unprocessed[0], unprocessed[1],
> unprocessed[2], data[offset]);
> break;
> default:
> -throw new AssertionError("Unprocessed length should
> be 1, 2, or 3: " + unprocessedLength);
> +throw new IllegalStateException("Unprocessed length
> should be 1, 2, or 3: " + unprocessedLength);
> }
> hash = mix32(k, hash);
> // Update the offset and length
>
That seems clearer, thanks.
This seems like the kind of code we might want fuzz test. It seems
quite unlikely otherwise we'd hit this case. I also wonder if
Thoughts?
I see in several places:
// Note: This fails to apply masking using 0xL to the seed.
Shouldn't this be in the Javadoc?
Gary
