Skip to content

Header fine tuning #1451

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

Merged
merged 1 commit into from
Oct 8, 2025
Merged

Header fine tuning #1451

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
116 changes: 47 additions & 69 deletions src/main/java/io/nats/client/impl/Headers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,17 @@ public Headers add(String key, Collection<String> values) {

// the add delegate
private Headers _add(String key, Collection<String> values) {
if (values != null) {
Checker checked = new Checker(key, values);
if (checked.hasValues()) {
// get values by key or compute empty if absent
// update the data length with the additional len
// update the lengthMap for the key to the old length plus the new length
List<String> currentSet = valuesMap.computeIfAbsent(key, k -> new ArrayList<>());
currentSet.addAll(checked.list);
dataLength += checked.len;
int oldLen = lengthMap.getOrDefault(key, 0);
lengthMap.put(key, oldLen + checked.len);
serialized = null; // since the data changed, clear this so it's rebuilt
}
CheckState checked = check(key, values);
Copy link
Contributor Author

@scottf scottf Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the check method is a refactor of all the checks and validations, plus bringing everything inline.

if (checked != null) {
// get values by key or compute empty if absent
// update the data length with the additional len
// update the lengthMap for the key to the old length plus the new length
List<String> currentSet = valuesMap.computeIfAbsent(key, k -> new ArrayList<>());
currentSet.addAll(checked.list);
dataLength += checked.len;
int oldLen = lengthMap.getOrDefault(key, 0);
lengthMap.put(key, oldLen + checked.len);
serialized = null; // since the data changed, clear this so it's rebuilt
}
return this;
}
Expand Down Expand Up @@ -221,19 +219,14 @@ public Headers put(Map<String, List<String>> map) {

// the put delegate
private Headers _put(String key, Collection<String> values) {
if (key == null || key.isEmpty()) {
throw new IllegalArgumentException("Key cannot be null or empty.");
}
if (values != null) {
Checker checked = new Checker(key, values);
if (checked.hasValues()) {
// update the data length removing the old length adding the new length
// put for the key
dataLength = dataLength - lengthMap.getOrDefault(key, 0) + checked.len;
valuesMap.put(key, checked.list);
lengthMap.put(key, checked.len);
serialized = null; // since the data changed, clear this so it's rebuilt
}
CheckState checked = check(key, values);
if (checked != null) {
// update the data length removing the old length adding the new length
// put for the key
dataLength = dataLength - lengthMap.getOrDefault(key, 0) + checked.len;
valuesMap.put(key, checked.list);
lengthMap.put(key, checked.len);
serialized = null; // since the data changed, clear this so it's rebuilt
}
return this;
}
Expand Down Expand Up @@ -514,66 +507,51 @@ public int serializeToArray(int destPosition, byte[] dest) {
return serializedLength();
}

/**
* Check the key to ensure it matches the specification for keys.
* @throws IllegalArgumentException if the key is null, empty or contains
* an invalid character
*/
static void checkKey(String key) {
// key cannot be null or empty and contain only printable characters except colon
static CheckState check(String key, Collection<String> values) {
if (key == null || key.isEmpty()) {
Copy link
Contributor Author

@scottf scottf Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the refactor does the key validation. Before, _put did this and then the old checker also did it, meaning it was checked twice. _add did it correctly in the checker.

throw new IllegalArgumentException(KEY_CANNOT_BE_EMPTY_OR_NULL);
}

int len = key.length();
for (int idx = 0; idx < len; idx++) {
// Check the key to ensure it matches the specification for keys.
int keyLen = key.length();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

instead of repeatedly calling .length

for (int idx = 0; idx < keyLen; idx++) {
char c = key.charAt(idx);
if (c < 33 || c > 126 || c == ':') {
throw new IllegalArgumentException(KEY_INVALID_CHARACTER + Integer.toHexString(c));
}
}
}

/**
* Check a non-null value if it matches the specification for values.
* @throws IllegalArgumentException if the value contains an invalid character
*/
static void checkValue(String val) {
// Like rfc822 section 3.1.2 (quoted in ADR 4)
// The field-body may be composed of any US-ASCII characters, except CR or LF.
for (int i = 0, len = val.length(); i < len; i++) {
int c = val.charAt(i);
if (c > 127 || c == 10 || c == 13) {
throw new IllegalArgumentException(VALUE_INVALID_CHARACTERS + Integer.toHexString(c));
}
if (values == null || values.isEmpty()) {
return null;
}
}

private static final class Checker {
List<String> list = new ArrayList<>();
int len = 0;

Checker(String key, Collection<String> values) {
checkKey(key);
if (!values.isEmpty()) {
for (String val : values) {
if (val != null) {
if (val.isEmpty()) {
list.add(val);
len += key.length() + 3; // for colon, cr, lf
}
else {
checkValue(val);
list.add(val);
len += key.length() + val.length() + 3; // for colon, cr, lf
for (String val : values) {
if (val != null) {
int valLen = val.length();
if (valLen > 0) {
// Check value to see if it matches the specification for values.
// Like rfc822 section 3.1.2 (quoted in ADR 4)
// The field-body may be composed of any US-ASCII characters, except CR or LF.
for (int i = 0; i < valLen; i++) {
int c = val.charAt(i);
if (c > 127 || c == 10 || c == 13) {
throw new IllegalArgumentException(VALUE_INVALID_CHARACTERS + Integer.toHexString(c));
}
}
}
list.add(val);
len += keyLen + valLen + 3; // 3 is for the colon, cr and lf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

length is zero if there were only null values

}
}
return len == 0 ? null : new CheckState(list, len);
}

static final class CheckState {
final List<String> list;
final int len;

boolean hasValues() {
return !list.isEmpty();
public CheckState(List<String> list, int len) {
this.list = list;
this.len = len;
}
}

Expand Down
26 changes: 14 additions & 12 deletions src/main/java/io/nats/client/impl/NatsJetStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.CompletableFuture;

Expand Down Expand Up @@ -191,20 +192,20 @@ private PublishAck processPublishResponse(Message resp, PublishOptions options)
}

private Headers mergePublishOptions(Headers headers, PublishOptions opts) {
if (opts == null) {
return headers;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if there aren't any opts, just short circuit. This is probably the most common way publish is called.

// never touch the user's original headers
Headers merged = headers == null ? null : new Headers(headers);

if (opts != null) {
merged = mergeNum(merged, EXPECTED_LAST_SEQ_HDR, opts.getExpectedLastSequence());
merged = mergeNum(merged, EXPECTED_LAST_SUB_SEQ_HDR, opts.getExpectedLastSubjectSequence());
merged = mergeString(merged, EXPECTED_LAST_SUB_SEQ_SUB_HDR, opts.getExpectedLastSubjectSequenceSubject());
merged = mergeString(merged, EXPECTED_LAST_MSG_ID_HDR, opts.getExpectedLastMsgId());
merged = mergeString(merged, EXPECTED_STREAM_HDR, opts.getExpectedStream());
merged = mergeString(merged, MSG_ID_HDR, opts.getMessageId());
merged = mergeString(merged, MSG_TTL_HDR, opts.getMessageTtl());
}

return merged;
merged = mergeNum(merged, EXPECTED_LAST_SEQ_HDR, opts.getExpectedLastSequence());
merged = mergeNum(merged, EXPECTED_LAST_SUB_SEQ_HDR, opts.getExpectedLastSubjectSequence());
merged = mergeString(merged, EXPECTED_LAST_SUB_SEQ_SUB_HDR, opts.getExpectedLastSubjectSequenceSubject());
merged = mergeString(merged, EXPECTED_LAST_MSG_ID_HDR, opts.getExpectedLastMsgId());
merged = mergeString(merged, EXPECTED_STREAM_HDR, opts.getExpectedStream());
merged = mergeString(merged, MSG_ID_HDR, opts.getMessageId());
return mergeString(merged, MSG_TTL_HDR, opts.getMessageTtl());
}

private Headers mergeNum(Headers h, String key, long value) {
Expand All @@ -219,7 +220,8 @@ private Headers _merge(Headers h, String key, String value) {
if (h == null) {
h = new Headers();
}
return h.add(key, value);
// this is always an internal header with one value per key
return h.put(key, Collections.singletonList(value));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

switch to put because there will only every be one of each key.

}

// ----------------------------------------------------------------------------------------------------
Expand Down
Loading