From 27eb10e2974ca518468f1649e0d29fc3ec8febe9 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 9 Sep 2021 17:25:59 +0200 Subject: Stop validating length of checksums Not needed, and the way we calculated hex string from a long did not always return 16 characters, so fixes that issue. Also log validation failures as warnings --- .../java/com/yahoo/vespa/config/PayloadChecksum.java | 6 ------ .../yahoo/vespa/config/protocol/RequestValidation.java | 18 +++++++++--------- .../yahoo/vespa/config/protocol/SlimeResponseData.java | 5 +---- .../com/yahoo/vespa/config/RequestValidationTest.java | 1 - .../vespa/config/protocol/JRTConfigRequestV3Test.java | 2 +- 5 files changed, 11 insertions(+), 21 deletions(-) (limited to 'config') diff --git a/config/src/main/java/com/yahoo/vespa/config/PayloadChecksum.java b/config/src/main/java/com/yahoo/vespa/config/PayloadChecksum.java index 17f486e15d1..bb3ac3f76f1 100644 --- a/config/src/main/java/com/yahoo/vespa/config/PayloadChecksum.java +++ b/config/src/main/java/com/yahoo/vespa/config/PayloadChecksum.java @@ -36,12 +36,6 @@ public class PayloadChecksum { public boolean valid() { if (checksum.equals("")) return true; // Empty checksum is ok (e.g. when running 'vespa-get-config') - if (type == Type.MD5 && checksum.length() != 32) { - return false; - } else if (type == Type.XXHASH64 && checksum.length() != 16) { - return false; - } - Matcher m = hexChecksumPattern.matcher(checksum); return m.matches(); } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/RequestValidation.java b/config/src/main/java/com/yahoo/vespa/config/protocol/RequestValidation.java index 9cd59798ecd..5737dc112e0 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/RequestValidation.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/RequestValidation.java @@ -1,16 +1,16 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.protocol; -import com.yahoo.vespa.config.PayloadChecksum; import com.yahoo.vespa.config.ConfigDefinition; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.ErrorCode; +import com.yahoo.vespa.config.PayloadChecksum; import java.util.logging.Logger; import java.util.regex.Matcher; import static com.yahoo.vespa.config.PayloadChecksum.Type.MD5; -import static java.util.logging.Level.INFO; +import static java.util.logging.Level.WARNING; /** * Static utility methods for verifying common request properties. @@ -23,31 +23,31 @@ public class RequestValidation { public static int validateRequest(JRTConfigRequest request) { ConfigKey key = request.getConfigKey(); if (!RequestValidation.verifyName(key.getName())) { - log.log(INFO, "Illegal name '" + key.getName() + "'"); + log.log(WARNING, "Illegal name '" + key.getName() + "'"); return ErrorCode.ILLEGAL_NAME; } if (!RequestValidation.verifyNamespace(key.getNamespace())) { - log.log(INFO, "Illegal name space '" + key.getNamespace() + "'"); + log.log(WARNING, "Illegal name space '" + key.getNamespace() + "'"); return ErrorCode.ILLEGAL_NAME_SPACE; } if (!(new PayloadChecksum(request.getRequestDefMd5(), MD5).valid())) { - log.log(INFO, "Illegal checksum '" + key.getNamespace() + "'"); + log.log(WARNING, "Illegal checksum '" + key.getNamespace() + "'"); return ErrorCode.ILLEGAL_DEF_MD5; // TODO: Use ILLEGAL_DEF_CHECKSUM } if (! request.getRequestConfigChecksums().valid()) { - log.log(INFO, "Illegal config checksum '" + request.getRequestConfigChecksums() + "'"); + log.log(WARNING, "Illegal config checksum '" + request.getRequestConfigChecksums() + "'"); return ErrorCode.ILLEGAL_CONFIG_MD5; // TODO: Use ILLEGAL_CONFIG_CHECKSUM } if (!RequestValidation.verifyGeneration(request.getRequestGeneration())) { - log.log(INFO, "Illegal generation '" + request.getRequestGeneration() + "'"); + log.log(WARNING, "Illegal generation '" + request.getRequestGeneration() + "'"); return ErrorCode.ILLEGAL_GENERATION; } if (!RequestValidation.verifyTimeout(request.getTimeout())) { - log.log(INFO, "Illegal timeout '" + request.getTimeout() + "'"); + log.log(WARNING, "Illegal timeout '" + request.getTimeout() + "'"); return ErrorCode.ILLEGAL_TIMEOUT; } if (!RequestValidation.verifyHostname(request.getClientHostName())) { - log.log(INFO, "Illegal client host name '" + request.getClientHostName() + "'"); + log.log(WARNING, "Illegal client host name '" + request.getClientHostName() + "'"); return ErrorCode.ILLEGAL_CLIENT_HOSTNAME; } return 0; diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeResponseData.java b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeResponseData.java index 965622adaa5..ca519fd7061 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeResponseData.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeResponseData.java @@ -78,11 +78,8 @@ class SlimeResponseData { : PayloadChecksum.empty(XXHASH64); } - PayloadChecksums getResponseConfigChecksums() { - PayloadChecksum responseConfigMd5 = getResponseConfigMd5(); - System.out.println(responseConfigMd5); - return PayloadChecksums.from(responseConfigMd5, getResponseConfigXxhash64()); + return PayloadChecksums.from(getResponseConfigMd5(), getResponseConfigXxhash64()); } CompressionInfo getCompressionInfo() { diff --git a/config/src/test/java/com/yahoo/vespa/config/RequestValidationTest.java b/config/src/test/java/com/yahoo/vespa/config/RequestValidationTest.java index 2b7fd9c350b..8e0c0d1671e 100644 --- a/config/src/test/java/com/yahoo/vespa/config/RequestValidationTest.java +++ b/config/src/test/java/com/yahoo/vespa/config/RequestValidationTest.java @@ -26,7 +26,6 @@ public class RequestValidationTest { assertTrue(new PayloadChecksum("e8f0c01c7c3dcb8d3f62d7ff777fce6b", MD5).valid()); assertTrue(new PayloadChecksum("e8f0c01c7c3dcb8d3f62d7ff777fce6B", MD5).valid()); assertTrue(new PayloadChecksum("e8f0c01c7c3dcb8d", XXHASH64).valid()); - assertFalse(new PayloadChecksum("aaaaaaaaaaaaaaaaaa", MD5).valid()); assertFalse(new PayloadChecksum("-8f0c01c7c3dcb8d3f62d7ff777fce6b", MD5).valid()); } diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java index 5a3110c9221..0a4fc3da6cb 100644 --- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java +++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java @@ -237,7 +237,7 @@ public class JRTConfigRequestV3Test { assertTrue(serverReq.validateParameters()); assertValidationFail(createReq("35#$#!$@#", defNamespace, hostname, configId, payloadChecksums, currentGeneration, timeout, trace)); assertValidationFail(createReq(defName, "abcd.o#$*(!&$", hostname, configId, payloadChecksums, currentGeneration, timeout, trace)); - assertValidationFail(createReq(defName, defNamespace, hostname, configId, PayloadChecksums.from("abcd", "1234"), currentGeneration, timeout, trace)); + assertValidationFail(createReq(defName, defNamespace, hostname, configId, PayloadChecksums.from("opnq", "1234"), currentGeneration, timeout, trace)); assertValidationFail(createReq(defName, defNamespace, hostname, configId, payloadChecksums, -34, timeout, trace)); assertValidationFail(createReq(defName, defNamespace, hostname, configId, payloadChecksums, currentGeneration, -23, trace)); assertValidationFail(createReq(defName, defNamespace, "", configId, payloadChecksums, currentGeneration, timeout, trace)); -- cgit v1.2.3