abhinav-phi commented on code in PR #2110:
URL: https://github.com/apache/stormcrawler/pull/2110#discussion_r3905606047
##########
external/warc/src/main/java/org/apache/stormcrawler/warc/WARCRecordFormat.java:
##########
@@ -93,19 +93,108 @@ public class WARCRecordFormat implements RecordFormat {
Pattern.compile("(?i)(?:Content-(?:Encoding|Length)|Transfer-Encoding)");
protected static final String X_HIDE_HEADER = "X-Crawler-";
+ /**
+ * Configuration key setting the algorithm used to compute the
WARC-Payload-Digest and
+ * WARC-Block-Digest fields. Supported values are {@value
#DIGEST_ALGORITHM_SHA1} (the default)
+ * and {@value #DIGEST_ALGORITHM_SHA256}.
+ *
+ * <p>Note: SHA-1 is the convention across the WARC ecosystem and
downstream tooling (CDX
+ * indexes, revisit record handling) may expect it. Change the default
deliberately, not
+ * casually.
+ */
+ public static final String DIGEST_ALGORITHM_PARAM =
"warc.digest.algorithm";
+
+ public static final String DIGEST_ALGORITHM_SHA1 = "sha1";
+
+ public static final String DIGEST_ALGORITHM_SHA256 = "sha256";
+
private static final Base32 base32 = new Base32();
- private static final String digestNoContent = getDigestSha1(new byte[0]);
protected final String protocolMDprefix;
+ /** JCA name of the message digest algorithm, e.g. "SHA-1". */
+ private final String digestJCAName;
+
+ /** Algorithm prefix of the WARC digest fields, e.g. "sha1:". */
+ private final String digestPrefix;
+
+ private final String digestNoContent;
+
public WARCRecordFormat(String protocolMDprefix) {
+ this(protocolMDprefix, DIGEST_ALGORITHM_SHA1);
+ }
+
+ public WARCRecordFormat(String protocolMDprefix, String digestAlgorithm) {
this.protocolMDprefix = protocolMDprefix;
+ this.digestJCAName = getDigestJCAName(digestAlgorithm);
+ this.digestPrefix =
digestJCAName.toLowerCase(Locale.ROOT).replace("-", "") + ":";
+ this.digestNoContent = getDigest(new byte[0]);
}
+ /**
+ * Resolve the configured digest algorithm to the JCA name of the message
digest. The value is
+ * matched case-insensitively and an optional hyphen is ignored, i.e.
"sha256",
+ * "SHA-256" etc. are all accepted.
+ *
+ * @throws IllegalArgumentException if the value is not a supported
algorithm
+ */
+ private static String getDigestJCAName(String digestAlgorithm) {
+ if (digestAlgorithm == null) {
+ return "SHA-1";
+ }
+ return switch
(digestAlgorithm.trim().toLowerCase(Locale.ROOT).replace("-", "")) {
+ case DIGEST_ALGORITHM_SHA1 -> "SHA-1";
+ case DIGEST_ALGORITHM_SHA256 -> "SHA-256";
+ default -> throw new IllegalArgumentException(
+ "Unsupported value ["
+ + digestAlgorithm
+ + "] for "
+ + DIGEST_ALGORITHM_PARAM
+ + ", supported algorithms: "
+ + DIGEST_ALGORITHM_SHA1
+ + ", "
+ + DIGEST_ALGORITHM_SHA256);
+ };
+ }
+
+ /**
+ * Compute the digest of the given bytes with the configured algorithm.
+ *
+ * @return digest in the form
"<algorithm>:<base32>", e.g.
+ * "sha1:..."
+ */
+ public String getDigest(byte[] bytes) {
+ MessageDigest md = DigestUtils.getDigest(digestJCAName);
+ return digestPrefix + base32.encodeAsString(md.digest(bytes));
Review Comment:
Agreed — good catch, thanks for flagging it before merge.
The spec backs this up: in ISO 28500 WARC 1.1 the digest fields are
`labelled-digest = algorithm ":" digest-value` with `digest-value = token`, and
the spec note explicitly says *"The grammar for digest-value disallows the
character = which is used for padding in Base32"*. So `sha256:...====` is
formally invalid, and as you say it's cheap to fix now and much less so once
padded digests are in existing archives.
Commit a6e24d04 strips the trailing padding: both `getDigest` methods now
route the Base32 output through a small helper that drops any trailing `=`. Two
notes on the impact:
- SHA-1 is unaffected: a 20-byte digest is exactly 32 Base32 characters, so
the default output is byte-for-byte identical to before. Only SHA-256 (52
characters + 4 `=`) changes.
- Unpadded Base32 is also what the rest of the ecosystem writes (e.g.
heritrix's WARC writer), so strict CDX tooling gets what it expects, while
lenient readers like jwarc are fine either way.
The test expectations were updated to the unpadded values and there is now
an explicit assertion that the produced digest value never contains `=` for
either algorithm; README and the configuration docs mention the unpadded
encoding too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
