github-actions[bot] commented on code in PR #4764:
URL: https://github.com/apache/iggy/pull/4764#discussion_r4482790754
##########
foreign/node/src/wire/message/header.utils.ts:
##########
@@ -165,6 +165,22 @@ export const serializeHeaderKey = (key: HeaderKey): Buffer
=> {
*/
export const serializeHeader = (key: HeaderKey, v: BinaryHeaderValue) => {
const bKey = serializeHeaderKey(key);
+ for (const [field, kind, bytes] of [
Review Comment:
simplification: The new loop duplicates the length and fixed-size checks
that `deserializeHeader()` runs at lines 332-361 and allocates a tuple array
for every header. Extract one `validateHeaderField(field, kind, length)` helper
and call it from both functions.
##########
foreign/node/src/wire/message/header.utils.ts:
##########
@@ -165,6 +165,22 @@ export const serializeHeaderKey = (key: HeaderKey): Buffer
=> {
*/
export const serializeHeader = (key: HeaderKey, v: BinaryHeaderValue) => {
const bKey = serializeHeaderKey(key);
+ for (const [field, kind, bytes] of [
+ ["key", key.kind, bKey],
+ ["value", v.kind, v.value],
+ ] as const) {
+ if (bytes.length < 1 || bytes.length > 255) {
Review Comment:
nit: The `serializeHeader()` JSDoc lists no `@throws`, though the function
now rejects two classes of input. Add an `@throws` line for the length bound
and the fixed-size rule.
--
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]
