diff --git a/build.gradle b/build.gradle index b24c8b754d3..b0f3ca2332b 100644 --- a/build.gradle +++ b/build.gradle @@ -9,7 +9,7 @@ apply plugin: "cz.habarta.typescript-generator" def checkstyleVersion = "8.33" def pmdVersion = "6.24.0" -def spotbugsVersion = "4.0.6" +def spotbugsVersion = "4.5.3" def jacocoVersion = "0.8.5" buildscript { @@ -21,7 +21,7 @@ buildscript { } dependencies { classpath "com.google.cloud.tools:appengine-gradle-plugin:2.3.0" - classpath "gradle.plugin.com.github.spotbugs.snom:spotbugs-gradle-plugin:4.4.3" + classpath "com.github.spotbugs.snom:spotbugs-gradle-plugin:5.0.6" classpath("cz.habarta.typescript-generator:typescript-generator-gradle-plugin:2.24.612") { exclude group: "org.gradle" } @@ -57,6 +57,7 @@ dependencies { implementation("com.google.guava:guava:30.1.1-jre") implementation(objectify) implementation("com.googlecode.owasp-java-html-sanitizer:owasp-java-html-sanitizer:20211018.2") + implementation("com.helger:ph-commons:9.5.1") // necessary to add SpotBugs suppression implementation("com.mailjet:mailjet-client:4.5.0") implementation("com.sendgrid:sendgrid-java:4.6.0") implementation("com.sun.jersey:jersey-client:1.19.4") diff --git a/src/client/java/teammates/client/scripts/DataBundleRegenerator.java b/src/client/java/teammates/client/scripts/DataBundleRegenerator.java index 452f84473f0..8a6255208d7 100644 --- a/src/client/java/teammates/client/scripts/DataBundleRegenerator.java +++ b/src/client/java/teammates/client/scripts/DataBundleRegenerator.java @@ -28,6 +28,9 @@ private DataBundleRegenerator() { private static void regenerateDataBundleJson(File folder) throws IOException { File[] listOfFiles = folder.listFiles(); + if (listOfFiles == null) { + listOfFiles = new File[] {}; + } for (File file : listOfFiles) { if (!file.getName().endsWith(".json") || NON_DATA_BUNDLE_JSON.contains(file.getName())) { continue; @@ -69,6 +72,9 @@ private static void saveFile(String filePath, String content) throws IOException private static void regenerateWebsiteDataJson() throws IOException { File[] listOfFiles = new File("./src/main/webapp/data").listFiles(); + if (listOfFiles == null) { + listOfFiles = new File[] {}; + } for (File file : listOfFiles) { if (!file.getName().endsWith(".json")) { continue; diff --git a/src/client/java/teammates/client/scripts/DataMigrationEntitiesBaseScript.java b/src/client/java/teammates/client/scripts/DataMigrationEntitiesBaseScript.java index b55445c2755..90f2207f0e8 100644 --- a/src/client/java/teammates/client/scripts/DataMigrationEntitiesBaseScript.java +++ b/src/client/java/teammates/client/scripts/DataMigrationEntitiesBaseScript.java @@ -19,6 +19,7 @@ import com.googlecode.objectify.cmd.Query; import teammates.client.connector.DatastoreClient; +import teammates.common.util.Const; import teammates.storage.entity.BaseEntity; import teammates.test.FileHelper; @@ -266,7 +267,7 @@ protected void log(String logLine) { Path logPath = Paths.get(BASE_LOG_URI + this.getClass().getSimpleName() + ".log"); try (OutputStream logFile = Files.newOutputStream(logPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.APPEND)) { - logFile.write((logLine + System.lineSeparator()).getBytes()); + logFile.write((logLine + System.lineSeparator()).getBytes(Const.ENCODING)); } catch (Exception e) { System.err.println("Error writing log line: " + logLine); System.err.println(e.getMessage()); diff --git a/src/client/java/teammates/client/scripts/statistics/FileStore.java b/src/client/java/teammates/client/scripts/statistics/FileStore.java index 1a1843ce516..1652fd2fe1e 100644 --- a/src/client/java/teammates/client/scripts/statistics/FileStore.java +++ b/src/client/java/teammates/client/scripts/statistics/FileStore.java @@ -31,6 +31,7 @@ import com.google.gson.stream.JsonWriter; import teammates.common.util.Config; +import teammates.common.util.Const; import teammates.common.util.StringHelper; import teammates.test.FileHelper; @@ -138,7 +139,7 @@ private static void saveEncryptedJsonToFile(String fileName, T object, Type try (OutputStream os = Files.newOutputStream(Paths.get(fileName))) { CipherOutputStream out = new CipherOutputStream(os, cipher); - JsonWriter writer = new JsonWriter(new OutputStreamWriter(out)); + JsonWriter writer = new JsonWriter(new OutputStreamWriter(out, Const.ENCODING)); getSerializer().toJson(object, typeOfObject, writer); writer.close(); out.close(); @@ -152,7 +153,7 @@ private static T parseEncryptedJsonFile(String fileName, CheckedFunction> errors = new HashMap<>(); try (Stream paths = Files.walk(Paths.get(TestProperties.TEST_DATA_FOLDER))) { @@ -52,6 +56,9 @@ public void checkTestDataValidity() throws IOException { errors.put(pathString, Collections.singletonList("Error reading file: " + e.getMessage())); return; } + if (path.getFileName() == null) { + return; + } String testPage = path.getFileName().toString().replace("E2ETest.json", ""); DataBundle dataBundle = JsonUtils.fromJson(jsonString, DataBundle.class); diff --git a/src/lnp/java/teammates/lnp/cases/BaseLNPTestCase.java b/src/lnp/java/teammates/lnp/cases/BaseLNPTestCase.java index 55c81f587c1..10caef0599a 100644 --- a/src/lnp/java/teammates/lnp/cases/BaseLNPTestCase.java +++ b/src/lnp/java/teammates/lnp/cases/BaseLNPTestCase.java @@ -321,6 +321,9 @@ protected void cleanupResults() throws IOException { .listFiles((d, s) -> { return s.contains(this.getClass().getSimpleName()); }); + if (fileList == null) { + fileList = new File[] {}; + } Arrays.sort(fileList, (a, b) -> { return b.getName().compareTo(a.getName()); }); diff --git a/src/lnp/java/teammates/lnp/cases/InstructorStudentCascadingUpdateLNPTest.java b/src/lnp/java/teammates/lnp/cases/InstructorStudentCascadingUpdateLNPTest.java index 8e32e31ff03..93525221226 100644 --- a/src/lnp/java/teammates/lnp/cases/InstructorStudentCascadingUpdateLNPTest.java +++ b/src/lnp/java/teammates/lnp/cases/InstructorStudentCascadingUpdateLNPTest.java @@ -61,8 +61,8 @@ public class InstructorStudentCascadingUpdateLNPTest extends BaseLNPTestCase { private static final double MEAN_RESP_TIME_LIMIT = 60; // To generate multiple csv files for multiple sections - private static int csvTestDataIndex; - private static LNPTestData testData; + private int csvTestDataIndex; + private LNPTestData testData; @Override protected LNPTestData getTestData() { diff --git a/src/main/java/teammates/common/datatransfer/questions/FeedbackContributionQuestionDetails.java b/src/main/java/teammates/common/datatransfer/questions/FeedbackContributionQuestionDetails.java index 44134f70439..a855454513b 100644 --- a/src/main/java/teammates/common/datatransfer/questions/FeedbackContributionQuestionDetails.java +++ b/src/main/java/teammates/common/datatransfer/questions/FeedbackContributionQuestionDetails.java @@ -373,5 +373,21 @@ public ContributionStatisticsEntry(int claimed, int perceived, Map getClaimedOthers() { + return claimedOthers; + } + + public int[] getPerceivedOthers() { + return perceivedOthers; + } } } diff --git a/src/main/java/teammates/common/datatransfer/questions/FeedbackRubricQuestionDetails.java b/src/main/java/teammates/common/datatransfer/questions/FeedbackRubricQuestionDetails.java index 06d383d7d07..4055c8af792 100644 --- a/src/main/java/teammates/common/datatransfer/questions/FeedbackRubricQuestionDetails.java +++ b/src/main/java/teammates/common/datatransfer/questions/FeedbackRubricQuestionDetails.java @@ -157,7 +157,8 @@ public List validateResponsesDetails(List respo } if (details.getAnswer().stream().anyMatch(choice -> - choice != RUBRIC_ANSWER_NOT_CHOSEN + choice == null + || choice != RUBRIC_ANSWER_NOT_CHOSEN && (choice < 0 || choice >= rubricChoices.size()))) { errors.add(RUBRIC_INVALID_ANSWER); } diff --git a/src/main/java/teammates/common/util/Config.java b/src/main/java/teammates/common/util/Config.java index ca26cac828e..16fea5d5a46 100644 --- a/src/main/java/teammates/common/util/Config.java +++ b/src/main/java/teammates/common/util/Config.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Properties; @@ -124,8 +125,10 @@ public final class Config { OAUTH2_CLIENT_SECRET = properties.getProperty("app.oauth2.client.secret"); ENABLE_DEVSERVER_LOGIN = Boolean.parseBoolean(properties.getProperty("app.enable.devserver.login", "true")); CAPTCHA_SECRET_KEY = properties.getProperty("app.captcha.secretkey"); - APP_ADMINS = Arrays.asList(properties.getProperty("app.admins", "").split(",")); - APP_MAINTAINERS = Arrays.asList(properties.getProperty("app.maintainers", "").split(",")); + APP_ADMINS = Collections.unmodifiableList( + Arrays.asList(properties.getProperty("app.admins", "").split(","))); + APP_MAINTAINERS = Collections.unmodifiableList( + Arrays.asList(properties.getProperty("app.maintainers", "").split(","))); SUPPORT_EMAIL = properties.getProperty("app.crashreport.email"); EMAIL_SENDEREMAIL = properties.getProperty("app.email.senderemail"); EMAIL_SENDERNAME = properties.getProperty("app.email.sendername"); diff --git a/src/main/java/teammates/common/util/Const.java b/src/main/java/teammates/common/util/Const.java index ede60001d7f..fe4a2366b8f 100644 --- a/src/main/java/teammates/common/util/Const.java +++ b/src/main/java/teammates/common/util/Const.java @@ -1,5 +1,7 @@ package teammates.common.util; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; @@ -27,7 +29,7 @@ public final class Const { public static final String UNKNOWN_INSTITUTION = "Unknown Institution"; public static final String DEFAULT_TIME_ZONE = "UTC"; - public static final String ENCODING = "UTF8"; + public static final Charset ENCODING = StandardCharsets.UTF_8; public static final Duration FEEDBACK_SESSIONS_SEARCH_WINDOW = Duration.ofDays(30); public static final Duration LOGS_RETENTION_PERIOD = Duration.ofDays(30); diff --git a/src/main/java/teammates/common/util/SanitizationHelper.java b/src/main/java/teammates/common/util/SanitizationHelper.java index ca65c7710d7..3dc7c385e36 100644 --- a/src/main/java/teammates/common/util/SanitizationHelper.java +++ b/src/main/java/teammates/common/util/SanitizationHelper.java @@ -1,6 +1,5 @@ package teammates.common.util; -import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import org.owasp.html.HtmlPolicyBuilder; @@ -34,7 +33,6 @@ public final class SanitizationHelper { .allowElements("quote", "ecode") .allowStyling() .toFactory(); - private static final Logger log = Logger.getLogger(); private SanitizationHelper() { // utility class @@ -128,13 +126,7 @@ public static String sanitizeForHtml(String unsanitizedString) { * Converts a string to be put in URL (replaces some characters). */ public static String sanitizeForUri(String uri) { - try { - return URLEncoder.encode(uri, Const.ENCODING).replaceAll("\\+", "%20"); - } catch (UnsupportedEncodingException wontHappen) { - log.warning("Unexpected UnsupportedEncodingException in " - + "SanitizationHelper.sanitizeForUri(" + uri + ", " + Const.ENCODING + ")"); - return uri; - } + return URLEncoder.encode(uri, Const.ENCODING).replaceAll("\\+", "%20"); } } diff --git a/src/main/java/teammates/common/util/StringHelper.java b/src/main/java/teammates/common/util/StringHelper.java index 1ea53044a1f..653b52a7d9a 100644 --- a/src/main/java/teammates/common/util/StringHelper.java +++ b/src/main/java/teammates/common/util/StringHelper.java @@ -89,7 +89,7 @@ public static String generateSignature(String data) { new SecretKeySpec(hexStringToByteArray(Config.ENCRYPTION_KEY), "HmacSHA1"); Mac mac = Mac.getInstance("HmacSHA1"); mac.init(signingKey); - byte[] value = mac.doFinal(data.getBytes()); + byte[] value = mac.doFinal(data.getBytes(Const.ENCODING)); return byteArrayToHexString(value); } catch (Exception e) { assert false; @@ -123,7 +123,7 @@ public static String encrypt(String value) { SecretKeySpec sks = new SecretKeySpec(hexStringToByteArray(Config.ENCRYPTION_KEY), "AES"); Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding"); cipher.init(Cipher.ENCRYPT_MODE, sks, cipher.getParameters()); - byte[] encrypted = cipher.doFinal(value.getBytes()); + byte[] encrypted = cipher.doFinal(value.getBytes(Const.ENCODING)); return byteArrayToHexString(encrypted); } catch (Exception e) { assert false; @@ -145,7 +145,7 @@ public static String decrypt(String message) throws InvalidParametersException { Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding"); cipher.init(Cipher.DECRYPT_MODE, sks); byte[] decrypted = cipher.doFinal(hexStringToByteArray(message)); - return new String(decrypted); + return new String(decrypted, Const.ENCODING); } catch (NumberFormatException | IllegalBlockSizeException | BadPaddingException e) { log.warning("Attempted to decrypt invalid ciphertext: " + message); throw new InvalidParametersException(e); diff --git a/src/main/java/teammates/logic/core/GoogleCloudTasksService.java b/src/main/java/teammates/logic/core/GoogleCloudTasksService.java index b8808428941..107d70370e3 100644 --- a/src/main/java/teammates/logic/core/GoogleCloudTasksService.java +++ b/src/main/java/teammates/logic/core/GoogleCloudTasksService.java @@ -1,7 +1,6 @@ package teammates.logic.core; import java.io.IOException; -import java.nio.charset.Charset; import java.time.Instant; import com.google.cloud.tasks.v2.AppEngineHttpRequest; @@ -49,7 +48,7 @@ public void addDeferredTask(TaskWrapper task, long countdownTime) { String requestBody = JsonUtils.toCompactJson(task.getRequestBody()); requestBuilder.putHeaders("Content-Type", "application/json; charset=UTF-8") .setRelativeUri(task.getWorkerUrl()) - .setBody(ByteString.copyFrom(requestBody, Charset.forName(Const.ENCODING))); + .setBody(ByteString.copyFrom(requestBody, Const.ENCODING)); } Task.Builder taskBuilder = Task.newBuilder().setAppEngineHttpRequest(requestBuilder.build()); diff --git a/src/main/java/teammates/logic/core/LocalTaskQueueService.java b/src/main/java/teammates/logic/core/LocalTaskQueueService.java index 29b1d2727a2..37501488999 100644 --- a/src/main/java/teammates/logic/core/LocalTaskQueueService.java +++ b/src/main/java/teammates/logic/core/LocalTaskQueueService.java @@ -3,7 +3,6 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -42,7 +41,7 @@ public void addDeferredTask(TaskWrapper task, long countdownTime) { if (task.getRequestBody() != null) { StringEntity entity = new StringEntity( - JsonUtils.toCompactJson(task.getRequestBody()), Charset.forName(Const.ENCODING)); + JsonUtils.toCompactJson(task.getRequestBody()), Const.ENCODING); post.setEntity(entity); } diff --git a/src/main/java/teammates/ui/servlets/DevServerLoginServlet.java b/src/main/java/teammates/ui/servlets/DevServerLoginServlet.java index d6a66a7420e..8dd38f60fd5 100644 --- a/src/main/java/teammates/ui/servlets/DevServerLoginServlet.java +++ b/src/main/java/teammates/ui/servlets/DevServerLoginServlet.java @@ -26,6 +26,8 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc if (nextUrl == null) { nextUrl = "/"; } + // Prevent HTTP response splitting + nextUrl = resp.encodeRedirectURL(nextUrl.replace("\r\n", "")); if (!Config.isDevServerLoginEnabled()) { resp.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY); resp.setHeader("Location", Const.WebPageURIs.LOGIN + "?nextUrl=" + nextUrl.replace("&", "%26")); @@ -66,6 +68,8 @@ public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOEx if (nextUrl == null) { nextUrl = "/"; } + // Prevent HTTP response splitting + nextUrl = resp.encodeRedirectURL(nextUrl.replace("\r\n", "")); resp.sendRedirect(nextUrl); } diff --git a/src/main/java/teammates/ui/servlets/LoginServlet.java b/src/main/java/teammates/ui/servlets/LoginServlet.java index aeeab71082e..11bd2b9ca5e 100644 --- a/src/main/java/teammates/ui/servlets/LoginServlet.java +++ b/src/main/java/teammates/ui/servlets/LoginServlet.java @@ -30,6 +30,8 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc if (nextUrl == null) { nextUrl = "/"; } + // Prevent HTTP response splitting + nextUrl = resp.encodeRedirectURL(nextUrl.replace("\r\n", "")); if (Config.isDevServerLoginEnabled()) { resp.setStatus(HttpStatus.SC_MOVED_PERMANENTLY); resp.setHeader("Location", "/devServerLogin?nextUrl=" + nextUrl.replace("&", "%26")); diff --git a/src/main/java/teammates/ui/servlets/OAuth2CallbackServlet.java b/src/main/java/teammates/ui/servlets/OAuth2CallbackServlet.java index c88291081ca..ba70ef9c4fd 100644 --- a/src/main/java/teammates/ui/servlets/OAuth2CallbackServlet.java +++ b/src/main/java/teammates/ui/servlets/OAuth2CallbackServlet.java @@ -80,8 +80,8 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO Map parsedResponse = JsonUtils.fromJson(userInfoResponse, new TypeToken>(){}.getType()); - String email = String.valueOf(parsedResponse.get("email")); - if (email != null) { + if (parsedResponse.containsKey("email")) { + String email = String.valueOf(parsedResponse.get("email")); googleId = email.replaceFirst("@gmail\\.com$", ""); } } catch (URISyntaxException | IOException | JsonSyntaxException e) { diff --git a/src/main/java/teammates/ui/webapi/DatastoreBackupAction.java b/src/main/java/teammates/ui/webapi/DatastoreBackupAction.java index 04efc9cbc02..53ed73ef353 100644 --- a/src/main/java/teammates/ui/webapi/DatastoreBackupAction.java +++ b/src/main/java/teammates/ui/webapi/DatastoreBackupAction.java @@ -3,7 +3,6 @@ import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; -import java.nio.charset.Charset; import java.time.Instant; import java.util.HashMap; import java.util.Map; @@ -64,12 +63,13 @@ public JsonResult execute() { // Documentation is wrong; the param name is output_url_prefix instead of outputUrlPrefix body.put("output_url_prefix", "gs://" + Config.BACKUP_GCS_BUCKETNAME + "/datastore-backups/" + timestamp); - StringEntity entity = new StringEntity(JsonUtils.toCompactJson(body), Charset.forName(Const.ENCODING)); + StringEntity entity = new StringEntity(JsonUtils.toCompactJson(body), Const.ENCODING); post.setEntity(entity); try (CloseableHttpClient client = HttpClients.createDefault(); CloseableHttpResponse resp = client.execute(post); - BufferedReader br = new BufferedReader(new InputStreamReader(resp.getEntity().getContent()))) { + BufferedReader br = new BufferedReader( + new InputStreamReader(resp.getEntity().getContent(), Const.ENCODING))) { String output = br.lines().collect(Collectors.joining(System.lineSeparator())); if (resp.getStatusLine().getStatusCode() == HttpStatus.SC_OK) { log.info("Backup request successful:" + System.lineSeparator() + output); diff --git a/src/main/java/teammates/ui/webapi/DeleteFeedbackResponseCommentAction.java b/src/main/java/teammates/ui/webapi/DeleteFeedbackResponseCommentAction.java index 8d2e37033b2..8761bb1e24d 100644 --- a/src/main/java/teammates/ui/webapi/DeleteFeedbackResponseCommentAction.java +++ b/src/main/java/teammates/ui/webapi/DeleteFeedbackResponseCommentAction.java @@ -61,8 +61,10 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { case INSTRUCTOR_RESULT: gateKeeper.verifyLoggedInUserPrivileges(userInfo); InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId()); - - if (instructor != null && frc.getCommentGiver().equals(instructor.getEmail())) { // giver, allowed by default + if (instructor == null) { + throw new UnauthorizedAccessException("Trying to access system using a non-existent instructor entity"); + } + if (frc.getCommentGiver().equals(instructor.getEmail())) { // giver, allowed by default return; } diff --git a/src/main/java/teammates/ui/webapi/GetAuthInfoAction.java b/src/main/java/teammates/ui/webapi/GetAuthInfoAction.java index c2fb4e9c989..63536b1e5c3 100644 --- a/src/main/java/teammates/ui/webapi/GetAuthInfoAction.java +++ b/src/main/java/teammates/ui/webapi/GetAuthInfoAction.java @@ -62,7 +62,7 @@ public JsonResult execute() { String csrfToken = StringHelper.encrypt(req.getSession().getId()); String existingCsrfToken = HttpRequestHelper.getCookieValueFromRequest(req, Const.SecurityConfig.CSRF_COOKIE_NAME); - if (csrfToken != null && csrfToken.equals(existingCsrfToken)) { + if (csrfToken.equals(existingCsrfToken)) { return new JsonResult(output); } Cookie csrfTokenCookie = new Cookie(Const.SecurityConfig.CSRF_COOKIE_NAME, csrfToken); diff --git a/src/main/java/teammates/ui/webapi/GetHasResponsesAction.java b/src/main/java/teammates/ui/webapi/GetHasResponsesAction.java index 6005fb55f33..9072a3df225 100644 --- a/src/main/java/teammates/ui/webapi/GetHasResponsesAction.java +++ b/src/main/java/teammates/ui/webapi/GetHasResponsesAction.java @@ -48,10 +48,6 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { } String courseId = getNonNullRequestParamValue(Const.ParamsNames.COURSE_ID); - if (courseId == null) { - return; - } - gateKeeper.verifyAccessible( logic.getInstructorForGoogleId(courseId, userInfo.getId()), logic.getCourse(courseId)); diff --git a/src/main/java/teammates/ui/webapi/UpdateFeedbackResponseCommentAction.java b/src/main/java/teammates/ui/webapi/UpdateFeedbackResponseCommentAction.java index 61205e16669..8314eff007c 100644 --- a/src/main/java/teammates/ui/webapi/UpdateFeedbackResponseCommentAction.java +++ b/src/main/java/teammates/ui/webapi/UpdateFeedbackResponseCommentAction.java @@ -81,7 +81,10 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { case INSTRUCTOR_RESULT: gateKeeper.verifyLoggedInUserPrivileges(userInfo); InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId()); - if (instructor != null && frc.getCommentGiver().equals(instructor.getEmail())) { // giver, allowed by default + if (instructor == null) { + throw new UnauthorizedAccessException("Trying to access system using a non-existent instructor entity"); + } + if (frc.getCommentGiver().equals(instructor.getEmail())) { // giver, allowed by default return; } gateKeeper.verifyAccessible(instructor, session, response.getGiverSection(), diff --git a/src/test/java/teammates/common/datatransfer/questions/FeedbackQuestionDetailsTest.java b/src/test/java/teammates/common/datatransfer/questions/FeedbackQuestionDetailsTest.java index c65a77af34c..27d4198bf5a 100644 --- a/src/test/java/teammates/common/datatransfer/questions/FeedbackQuestionDetailsTest.java +++ b/src/test/java/teammates/common/datatransfer/questions/FeedbackQuestionDetailsTest.java @@ -13,14 +13,12 @@ public class FeedbackQuestionDetailsTest extends BaseTestCase { public void testEquals() { ______TS("Same object with different references, should be same"); - FeedbackQuestionDetails ftqd1 = new FeedbackTextQuestionDetails("text question"); + FeedbackTextQuestionDetails ftqd1 = new FeedbackTextQuestionDetails("text question"); FeedbackQuestionDetails ftqd2 = ftqd1; assertEquals(ftqd1, ftqd2); ______TS("One input is null, should be different"); - ftqd1 = new FeedbackTextQuestionDetails("text question"); - ftqd2 = null; - assertNotEquals(ftqd1, ftqd2); + assertNotEquals(ftqd1, null); ______TS("Different classes, should be different"); ftqd1 = new FeedbackTextQuestionDetails("text question"); @@ -33,7 +31,7 @@ public void testEquals() { assertNotEquals(ftqd1, ftqd2); ftqd2 = new FeedbackTextQuestionDetails("first question"); - ((FeedbackTextQuestionDetails) ftqd1).setRecommendedLength(50); + ftqd1.setRecommendedLength(50); assertNotEquals(ftqd1, ftqd2); ______TS("All attributes are same, should be same"); diff --git a/src/test/java/teammates/common/datatransfer/questions/FeedbackRubricQuestionDetailsTest.java b/src/test/java/teammates/common/datatransfer/questions/FeedbackRubricQuestionDetailsTest.java index 982f2a7a2bc..ae7f532fd07 100644 --- a/src/test/java/teammates/common/datatransfer/questions/FeedbackRubricQuestionDetailsTest.java +++ b/src/test/java/teammates/common/datatransfer/questions/FeedbackRubricQuestionDetailsTest.java @@ -100,6 +100,9 @@ public void testValidateResponseDetails_invalidAnswer_shouldReturnNonEmptyErrorL responseDetails.setAnswer(Arrays.asList(0, 1, 0)); assertFalse(rubricQuestionDetails.validateResponsesDetails(Collections.singletonList(responseDetails), 0).isEmpty()); + + responseDetails.setAnswer(Arrays.asList(0, null, 0)); + assertFalse(rubricQuestionDetails.validateResponsesDetails(Collections.singletonList(responseDetails), 0).isEmpty()); } @Test diff --git a/src/test/java/teammates/common/util/StringHelperTest.java b/src/test/java/teammates/common/util/StringHelperTest.java index 16fd16a9c3d..4ad3a5446c7 100644 --- a/src/test/java/teammates/common/util/StringHelperTest.java +++ b/src/test/java/teammates/common/util/StringHelperTest.java @@ -107,7 +107,7 @@ private static String encryptWithoutSpecifyingAlgorithmParams(String plaintext) SecretKeySpec sks = new SecretKeySpec(StringHelper.hexStringToByteArray(Config.ENCRYPTION_KEY), "AES"); Cipher cipher = Cipher.getInstance("AES"); cipher.init(Cipher.ENCRYPT_MODE, sks, cipher.getParameters()); - byte[] encrypted = cipher.doFinal(plaintext.getBytes()); + byte[] encrypted = cipher.doFinal(plaintext.getBytes(Const.ENCODING)); return StringHelper.byteArrayToHexString(encrypted); } @@ -116,7 +116,7 @@ private static String generateSignature(String data) throws Exception { new SecretKeySpec(StringHelper.hexStringToByteArray(Config.ENCRYPTION_KEY), "HmacSHA1"); Mac mac = Mac.getInstance("HmacSHA1"); mac.init(signingKey); - byte[] value = mac.doFinal(data.getBytes()); + byte[] value = mac.doFinal(data.getBytes(Const.ENCODING)); return StringHelper.byteArrayToHexString(value); } diff --git a/src/test/java/teammates/test/AbstractBackDoor.java b/src/test/java/teammates/test/AbstractBackDoor.java index d8573e2806a..58e9699a1d0 100644 --- a/src/test/java/teammates/test/AbstractBackDoor.java +++ b/src/test/java/teammates/test/AbstractBackDoor.java @@ -5,7 +5,6 @@ import java.io.InputStreamReader; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.Charset; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -156,7 +155,8 @@ private ResponseBodyAndCode executeRequest( String responseBody = null; HttpEntity entity = response.getEntity(); if (entity != null) { - try (BufferedReader br = new BufferedReader(new InputStreamReader(entity.getContent()))) { + try (BufferedReader br = new BufferedReader( + new InputStreamReader(entity.getContent(), Const.ENCODING))) { responseBody = br.lines().collect(Collectors.joining(System.lineSeparator())); } } @@ -180,7 +180,7 @@ private static HttpPost createPostRequest(String url, Map params HttpPost post = new HttpPost(createBasicUri(url, params)); if (body != null) { - StringEntity entity = new StringEntity(body, Charset.forName(Const.ENCODING)); + StringEntity entity = new StringEntity(body, Const.ENCODING); post.setEntity(entity); } @@ -191,7 +191,7 @@ private static HttpPut createPutRequest(String url, Map params, HttpPut put = new HttpPut(createBasicUri(url, params)); if (body != null) { - StringEntity entity = new StringEntity(body, Charset.forName(Const.ENCODING)); + StringEntity entity = new StringEntity(body, Const.ENCODING); put.setEntity(entity); } diff --git a/src/test/java/teammates/test/MockHttpServletRequest.java b/src/test/java/teammates/test/MockHttpServletRequest.java index 8270eb354da..82de2b44fae 100644 --- a/src/test/java/teammates/test/MockHttpServletRequest.java +++ b/src/test/java/teammates/test/MockHttpServletRequest.java @@ -28,6 +28,8 @@ import javax.servlet.http.HttpUpgradeHandler; import javax.servlet.http.Part; +import teammates.common.util.Const; + /** * Mocks {@link HttpServletRequest} for testing purpose. * @@ -420,8 +422,8 @@ public int getServerPort() { @Override public BufferedReader getReader() { - byte[] bytes = this.body == null ? new byte[] {} : this.body.getBytes(); - return new BufferedReader(new InputStreamReader(new ByteArrayInputStream(bytes))); + byte[] bytes = this.body == null ? new byte[] {} : this.body.getBytes(Const.ENCODING); + return new BufferedReader(new InputStreamReader(new ByteArrayInputStream(bytes), Const.ENCODING)); } public void setBody(String body) { diff --git a/src/test/java/teammates/test/MockHttpServletResponse.java b/src/test/java/teammates/test/MockHttpServletResponse.java index de0d06ddacb..8001039860f 100644 --- a/src/test/java/teammates/test/MockHttpServletResponse.java +++ b/src/test/java/teammates/test/MockHttpServletResponse.java @@ -12,6 +12,8 @@ import org.apache.http.HttpStatus; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * Mocks {@link HttpServletResponse} for testing purpose. * @@ -148,6 +150,7 @@ public ServletOutputStream getOutputStream() { } @Override + @SuppressFBWarnings("DM_DEFAULT_ENCODING") public PrintWriter getWriter() { return new PrintWriter(System.out); } diff --git a/static-analysis/teammates-spotbugs.xml b/static-analysis/teammates-spotbugs.xml index 119b01a1b1d..4a5aaddca27 100644 --- a/static-analysis/teammates-spotbugs.xml +++ b/static-analysis/teammates-spotbugs.xml @@ -3,39 +3,249 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +