Skip to content

Fix detached buffer#991

Merged
xeioex merged 4 commits into
nginx:masterfrom
xeioex:fix_detached_buffer
Nov 21, 2025
Merged

Fix detached buffer#991
xeioex merged 4 commits into
nginx:masterfrom
xeioex:fix_detached_buffer

Conversation

@xeioex
Copy link
Copy Markdown
Contributor

@xeioex xeioex commented Nov 20, 2025

No description provided.

@xeioex xeioex force-pushed the fix_detached_buffer branch from 542b62c to e49b938 Compare November 20, 2025 04:35
@xeioex xeioex marked this pull request as ready for review November 20, 2025 06:17
@xeioex xeioex force-pushed the fix_detached_buffer branch from e49b938 to 43d91cf Compare November 20, 2025 17:03
@VadimZhestikov
Copy link
Copy Markdown
Contributor

njs_buffer.c:

static njs_int_t
njs_buffer_byte_length(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
njs_index_t unused, njs_value_t *retval)
{
size_t size;
njs_value_t *value;
const njs_buffer_encoding_t *encoding;

value = njs_arg(args, nargs, 1);

switch (value->type) {
case NJS_TYPED_ARRAY:
    njs_set_number(retval, njs_typed_array(value)->byte_length);
    return NJS_OK;

Should we set retval=0 here in case of detached buffer?

Need we add similar checks for detached array after some other njs_typed_array() invocations?

@xeioex xeioex force-pushed the fix_detached_buffer branch from 43d91cf to 16e085b Compare November 21, 2025 00:30
Comment thread src/njs_typed_array.c Outdated
Copy link
Copy Markdown
Contributor

@VadimZhestikov VadimZhestikov left a comment

Choose a reason for hiding this comment

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

Otherwise looks good.

@VadimZhestikov VadimZhestikov self-requested a review November 21, 2025 19:34
VadimZhestikov
VadimZhestikov previously approved these changes Nov 21, 2025
Copy link
Copy Markdown
Contributor

@VadimZhestikov VadimZhestikov left a comment

Choose a reason for hiding this comment

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

Looks good

@xeioex xeioex merged commit 422b4c3 into nginx:master Nov 21, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants