Skip to content

#1998 WARC writer: WARC-Protocol header to follow WARC field proposals #2034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sebastian-nagel wants to merge 1 commit into
base: main
Choose a base branch
from
+37 −7
Open

#1998 WARC writer: WARC-Protocol header to follow WARC field proposals #2034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/src/main/java/org/apache/stormcrawler/protocol/ProtocolResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ public class ProtocolResponse {
*/
public static final String PROTOCOL_VERSIONS_KEY = "_protocol_versions_";

/**
* Key which holds the SSL/TLS cipher suites. For requests sent over http:// the value may be
* null.
*/
public static final String CIPHER_SUITES_KEY = "_cipher_suites_";

@dpol1 dpol1 Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why plural? One handshake gives one suite and the WARC header is singular. Once this key ships in persisted metadata we can't rename it - is _cipher_suite_ better while we still can?


/**
* Metadata key which holds a boolean value in metadata whether the response content is trimmed
* or not.
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,10 +669,11 @@ public Response intercept(Interceptor.Chain chain) throws IOException {
.getBytes(StandardCharsets.ISO_8859_1));

final StringBuilder protocols = new StringBuilder(response.protocol().toString());
String cipherSuite = null;
final Handshake handshake = connection.handshake();
if (handshake != null) {
protocols.append(',').append(handshake.tlsVersion());

@dpol1 dpol1 Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this write, TLS_1_3 or tls/1.3? The registry only lists the tls/x.y forms - is the enum name close enough for the proposal, or do we want a mapping here?

protocols.append(',').append(handshake.cipherSuite());
cipherSuite = handshake.cipherSuite().toString();
}

// returns a modified version of the response
Expand All @@ -686,6 +687,7 @@ public Response intercept(Interceptor.Chain chain) throws IOException {
.header(ProtocolResponse.RESPONSE_IP_KEY, ipAddress)
.header(ProtocolResponse.REQUEST_TIME_KEY, Long.toString(startFetchTime))
.header(ProtocolResponse.PROTOCOL_VERSIONS_KEY, protocols.toString())
.header(ProtocolResponse.CIPHER_SUITES_KEY, cipherSuite)

@jnioche jnioche Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if cipherSuite is null?

@sebastian-nagel sebastian-nagel Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARC writer checks for null anyway, because another protocol implementation could have been used which does not collect the necessary information. If it's not available, it does not add the WARC-Cipher-Suite header.

But I can change the code, so that no header is added, if no Cipher suite was used (connection over http://).

@dpol1 dpol1 Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't Response.Builder.header() reject a null value though? If the handshake is null (plain http) I don't see how this survives the interceptor - have you tried a plain http:// fetch with http.store.headers=true? That's the WARC setup, so worth checking before the writer's null check even comes into play. Maybe worth a small test on the interceptor too, I don't think anything covers it right now?

.build();
}
}
Expand Down
9 changes: 8 additions & 1 deletion external/warc/src/main/java/org/apache/stormcrawler/warc/WARCRecordFormat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,14 @@ public byte[] format(Tuple tuple) {
metadata.getFirstValue(
ProtocolResponse.PROTOCOL_VERSIONS_KEY, this.protocolMDprefix);
if (protocolVersions != null) {
buffer.append("WARC-Protocol: ").append(protocolVersions).append(CRLF);
for (String val : StringUtils.split(protocolVersions, ',')) {
buffer.append("WARC-Protocol: ").append(val).append(CRLF);
}
}
final String cipherSuites =
metadata.getFirstValue(ProtocolResponse.CIPHER_SUITES_KEY, this.protocolMDprefix);
if (cipherSuites != null) {
buffer.append("WARC-Cipher-Suite: ").append(cipherSuites).append(CRLF);
}

buffer.append("WARC-Payload-Digest").append(": ").append(payloadDigest).append(CRLF);
Expand Down
11 changes: 10 additions & 1 deletion external/warc/src/test/java/org/apache/stormcrawler/warc/WARCHdfsBoltTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,13 @@ void testHttp2() throws IOException {
assertTrue(
response.headers().first("WARC-Protocol").isPresent(),
"WARC response record is expected to include WARC header \"WARC-Protocol\"");
assertEquals(
2,
response.headers().all("WARC-Protocol").size(),
"WARC response record is expected to include WARC header \"WARC-Protocol\"");
assertTrue(
response.headers().first("WARC-Cipher-Suite").isPresent(),
"WARC response record is expected to include WARC header \"WARC-Cipher-Suite\"");
assertTrue(
response.headers().first("WARC-IP-Address").isPresent(),
"WARC response record is expected to include WARC header \"WARC-IP-Address\"");
Expand Down Expand Up @@ -211,7 +218,9 @@ private Tuple getPage(String httpVersionString) {
+ "Connection: close\r\n\r\n");
metadata.addValue(
protocolMDprefix + ProtocolResponse.PROTOCOL_VERSIONS_KEY,
httpVersionString + ",TLS_1_3,TLS_AES_256_GCM_SHA384");
httpVersionString + ",TLS_1_3");
metadata.addValue(
protocolMDprefix + ProtocolResponse.CIPHER_SUITES_KEY, "TLS_AES_256_GCM_SHA384");
metadata.addValue(protocolMDprefix + ProtocolResponse.RESPONSE_IP_KEY, "123.123.123.123");
Tuple tuple = mock(Tuple.class);
when(tuple.getBinaryByField("content")).thenReturn(content);
Expand Down
14 changes: 10 additions & 4 deletions external/warc/src/test/java/org/apache/stormcrawler/warc/WARCRecordFormatTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,9 @@ void testReplaceHttpVersion() {
+ "Content-Encoding: gzip\r\n"
+ "Content-Length: 26\r\n"
+ "Connection: close");
metadata.addValue(protocolMDprefix + ProtocolResponse.PROTOCOL_VERSIONS_KEY, "h2,TLS_1_3");
metadata.addValue(
protocolMDprefix + ProtocolResponse.PROTOCOL_VERSIONS_KEY,
"h2,TLS_1_3,TLS_AES_256_GCM_SHA384");
protocolMDprefix + ProtocolResponse.CIPHER_SUITES_KEY, "TLS_AES_256_GCM_SHA384");
metadata.addValue(protocolMDprefix + ProtocolResponse.RESPONSE_IP_KEY, "123.123.123.123");
Tuple tuple = mock(Tuple.class);
when(tuple.getBinaryByField("content")).thenReturn(content);
Expand All @@ -246,8 +246,14 @@ void testReplaceHttpVersion() {
statusLine.matches("^HTTP/1\\.[01] .*"),
"WARC response record: HTTP status line must start with HTTP/1.1 or HTTP/1.0");
assertTrue(
headersPayload[0].contains("\r\nWARC-Protocol: "),
"WARC response record is expected to include WARC header \"WARC-Protocol\"");
headersPayload[0].contains("\r\nWARC-Protocol: h2\r\n"),
"WARC response record is expected to include a WARC header \"WARC-Protocol: h2\"");
assertTrue(
headersPayload[0].contains("\r\nWARC-Protocol: TLS_1_3\r\n"),
"WARC response record is expected to include a WARC header \"WARC-Protocol: TLS_1_3\"");
assertTrue(
headersPayload[0].contains("\r\nWARC-Cipher-Suite: "),
"WARC response record is expected to include WARC header \"WARC-Cipher-Suite\"");
assertTrue(
headersPayload[0].contains("\r\nWARC-IP-Address: "),
"WARC response record is expected to include WARC header \"WARC-IP-Address\"");
Expand Down
Loading