Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/pr-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
review:
needs: check
if: needs.check.outputs.allowed == 'true'
uses: adobe/aio-reusable-workflows/.github/workflows/pr-review.yml@main
uses: adobe/aio-reusable-workflows/.github/workflows/pr-review.yml@fix/pr-review-pedantic-and-human-replies

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pinning the reusable workflow to a feature branch (fix/pr-review-pedantic-and-human-replies) rather than a stable ref like main or a tagged version means any force-push or deletion of that branch could break CI. Ensure this is reverted to main or a versioned tag once the fix is merged.

Suggested change
uses: adobe/aio-reusable-workflows/.github/workflows/pr-review.yml@fix/pr-review-pedantic-and-human-replies
uses: adobe/aio-reusable-workflows/.github/workflows/pr-review.yml@main

with:
pr_number: ${{ needs.check.outputs.pr_number }}
head_sha: ${{ needs.check.outputs.head_sha }}
Expand Down
34 changes: 33 additions & 1 deletion lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,40 @@ function formatAjvErrors (errors) {
return stringErrors
}

/**
* Computes the TTL in seconds from an expiry date or a number of seconds.
* Returns undefined if no expiry is provided.
*
* @private
* @param {Date|number} expiry a Date object or seconds from now
* @returns {number|undefined} TTL in seconds
*/
function computeTTL (expiry) {
if (expiry === undefined || expiry === null) return undefined
if (expiry instanceof Date) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Re-raised] When a Date in the past is provided, this returns a negative TTL, which will likely cause silent bugs or unexpected behavior downstream. Consider returning 0 (or throwing) for expired dates.

Suggested change
if (expiry instanceof Date) {
if (expiry instanceof Date) {
const ttl = Math.floor((expiry.getTime() - Date.now()) / 1000)
return ttl > 0 ? ttl : 0
}

return Math.floor((expiry.getTime() - Date.now()) / 1000)
Comment on lines +129 to +131

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Re-raised] When a Date in the past is provided, this returns a negative TTL, which will likely cause silent bugs or unexpected behavior downstream. Consider returning 0 (or throwing) for expired dates.

Suggested change
if (expiry === undefined || expiry === null) return undefined
if (expiry instanceof Date) {
return Math.floor((expiry.getTime() - Date.now()) / 1000)
if (expiry instanceof Date) {
const ttl = Math.floor((expiry.getTime() - Date.now()) / 1000)
return ttl > 0 ? ttl : 0
}

}
return expiry
}

/**
* Truncates a value to the max allowed byte size for state storage.
*
* @private
* @param {string} value the value to sanitize
* @param {number} maxBytes max allowed size in bytes
* @returns {string} the value, truncated if necessary
*/
function sanitizeValue (value, maxBytes) {
const encoded = Buffer.from(value)
if (encoded.length <= maxBytes) return value
Comment on lines +143 to +146

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Re-raised] Slicing a UTF-8 Buffer at an arbitrary byte boundary can split a multi-byte character, producing an invalid UTF-8 sequence and corrupting the last character. Also, the missing type guard means non-string values will cause Buffer.from to throw or behave unexpectedly.

Suggested change
*/
function sanitizeValue (value, maxBytes) {
const encoded = Buffer.from(value)
if (encoded.length <= maxBytes) return value
function sanitizeValue (value, maxBytes) {
if (typeof value !== 'string') throw new TypeError('sanitizeValue: value must be a string')
const encoded = Buffer.from(value)
if (encoded.length <= maxBytes) return value
let truncated = encoded.slice(0, maxBytes).toString('utf8')
while (Buffer.byteLength(truncated) > maxBytes) truncated = truncated.slice(0, -1)
return truncated
}

return encoded.slice(0, maxBytes).toString()
}

Comment on lines +145 to +149

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Re-raised] Slicing a UTF-8 Buffer at an arbitrary byte boundary can split a multi-byte character, producing an invalid UTF-8 sequence and corrupting the last character. Also, the missing type guard means non-string values will cause Buffer.from to throw or behave unexpectedly.

Suggested change
const encoded = Buffer.from(value)
if (encoded.length <= maxBytes) return value
return encoded.slice(0, maxBytes).toString()
}
function sanitizeValue (value, maxBytes) {
if (typeof value !== 'string') throw new TypeError('sanitizeValue: value must be a string')
const encoded = Buffer.from(value)
if (encoded.length <= maxBytes) return value
let truncated = encoded.slice(0, maxBytes).toString('utf8')
while (Buffer.byteLength(truncated) > maxBytes) truncated = truncated.slice(0, -1)
return truncated
}

module.exports = {
withHiddenFields,
isInternalToAdobeRuntime,
formatAjvErrors
formatAjvErrors,
computeTTL,
sanitizeValue
}
Loading