Skip to content

Commit

Permalink
Add null guards (#4542)
Browse files Browse the repository at this point in the history
While the `@NotNull` decorator informs developers that the correct usage
is that the decorated variables should not be `null`, it does not enforce
such in any way.
This leads to cases where an incorrectly fed `null` could propagate far
away before being detected (such as via `NullPointerException`s).
This commit implements some `null` guards where needed for a fail-fast
treatment of `null` inputs.
  • Loading branch information
pingpingy1 committed Feb 12, 2024
1 parent bbaef9d commit 500e28c
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
import java.util.Objects;

/**
* ThreadFactory to be used throughout OpenGrok to make sure all threads have common prefix.
Expand All @@ -45,7 +46,7 @@ public OpenGrokThreadFactory(String name) {

@Override
public Thread newThread(@NotNull Runnable runnable) {
Thread thread = Executors.defaultThreadFactory().newThread(runnable);
Thread thread = Executors.defaultThreadFactory().newThread(Objects.requireNonNull(runnable, "runnable"));
thread.setName(PREFIX + threadPrefix + thread.getId());
return thread;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.io.Reader;
import java.io.StringReader;
import java.util.List;
import java.util.Objects;

import org.jetbrains.annotations.NotNull;
import org.opengrok.indexer.util.IOUtils;
Expand All @@ -47,7 +48,7 @@ public int read(char @NotNull [] cbuf, int off, int len) throws IOException {
if (input == null) {
input = createInternalReader();
}
return input.read(cbuf, off, len);
return input.read(Objects.requireNonNull(cbuf, "cbuf"), off, len);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -101,7 +102,7 @@ public enum IndexCheckMode {
* @param configuration configuration based on which to perform the check
*/
public IndexCheck(@NotNull Configuration configuration) {
this(configuration, null);
this(configuration, null); // configuration is guarded against null inside this constructor
}

/**
Expand All @@ -111,7 +112,7 @@ public IndexCheck(@NotNull Configuration configuration) {
* on whether projects are enabled in the configuration.
*/
public IndexCheck(@NotNull Configuration configuration, Collection<String> projectNames) {
this.configuration = configuration;
this.configuration = Objects.requireNonNull(configuration, "configuration");
if (projectNames != null) {
this.projectNames.addAll(projectNames);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2124,7 +2124,7 @@ private static class CountingWriter extends Writer {

@Override
public void write(@NotNull char[] chars, int off, int len) throws IOException {
out.write(chars, off, len);
out.write(Objects.requireNonNull(chars), off, len);
count += len;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* CDDL HEADER START
*
* The contents of this file are subject to the terms of the
* Common Development and Distribution License (the "License").
* You may not use this file except in compliance with the License.
*
* See LICENSE.txt included in this distribution for the specific
* language governing permissions and limitations under the License.
*
* When distributing Covered Code, include this CDDL HEADER in each
* file and include the License file at LICENSE.txt.
* If applicable, add the following below this CDDL HEADER, with the
* fields enclosed by brackets "[]" replaced with your own identifying
* information: Portions Copyright [yyyy] [name of copyright owner]
*
* CDDL HEADER END
*/

/*
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* Portions Copyright (c) 2024, Heewon Lee <heewon.lee@kaist.ac.kr>.
*/
package org.opengrok.indexer.configuration;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertThrows;

public class OpenGrokThreadFactoryTest {
@Test
void testNullRunnable() throws Exception {
assertThrows(NullPointerException.class, () -> {
OpenGrokThreadFactory factory = new OpenGrokThreadFactory("");
factory.newThread(null);
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* CDDL HEADER START
*
* The contents of this file are subject to the terms of the
* Common Development and Distribution License (the "License").
* You may not use this file except in compliance with the License.
*
* See LICENSE.txt included in this distribution for the specific
* language governing permissions and limitations under the License.
*
* When distributing Covered Code, include this CDDL HEADER in each
* file and include the License file at LICENSE.txt.
* If applicable, add the following below this CDDL HEADER, with the
* fields enclosed by brackets "[]" replaced with your own identifying
* information: Portions Copyright [yyyy] [name of copyright owner]
*
* CDDL HEADER END
*/

/*
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* Portions Copyright (c) 2024, Heewon Lee <heewon.lee@kaist.ac.kr>.
*/
package org.opengrok.indexer.history;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertThrows;

public class HistoryReaderTest {
@Test
void testNullCbuf() throws Exception {
assertThrows(NullPointerException.class, () -> {
HistoryReader historyReader = new HistoryReader(new History());
historyReader.read(null, 0, 0);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -255,4 +255,12 @@ void testMultipleTestsWithSameInstance() throws Exception {
}
}
}

@Test
void testNullConfiguration() throws Exception {
assertThrows(NullPointerException.class, () -> {
new IndexCheck(null);
}
);
}
}

0 comments on commit 500e28c

Please sign in to comment.