Skip to content
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

Jump to bottom

Fix memory usage when reading input #94

Merged
merged 1 commit into from
Dec 17, 2022
Merged

Fix memory usage when reading input #94

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 4 additions & 33 deletions ngx_http_zip_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "ngx_http_zip_file.h"
#include "ngx_http_zip_headers.h"

static size_t ngx_chain_length(ngx_chain_t *chain_link);
static ngx_chain_t *ngx_chain_last_link(ngx_chain_t *chain_link);
static ngx_int_t ngx_http_zip_discard_chain(ngx_http_request_t *r,
ngx_chain_t *in);
Expand Down Expand Up @@ -95,15 +94,6 @@ ngx_module_t ngx_http_zip_module = {
NGX_MODULE_V1_PADDING
};

static size_t ngx_chain_length(ngx_chain_t *chain_link)
{
size_t len;
for (len=0; chain_link; chain_link = chain_link->next) {
len += chain_link->buf->last - chain_link->buf->pos;
}
return len;
}

static ngx_chain_t *ngx_chain_last_link(ngx_chain_t *chain_link)
{
ngx_chain_t *cl;
Expand Down Expand Up @@ -139,34 +129,14 @@ ngx_http_zip_ranges_intersect(ngx_http_zip_range_t *range1, ngx_http_zip_range_t
static ngx_int_t ngx_http_zip_copy_unparsed_request(ngx_http_request_t *r,
ngx_chain_t *in, ngx_http_zip_ctx_t *ctx)
{
ngx_str_t *old_unparsed_request;
ngx_chain_t *chain_link;
size_t len, offset = 0;

old_unparsed_request = ctx->unparsed_request;

len = ngx_chain_length(in);

if (old_unparsed_request != NULL)
len += old_unparsed_request->len;

if ((ctx->unparsed_request = ngx_palloc(r->pool, sizeof(ngx_str_t))) == NULL
|| (ctx->unparsed_request->data = ngx_palloc(r->pool, len)) == NULL) {
return NGX_ERROR;
}

if (old_unparsed_request != NULL) {
ngx_memcpy(ctx->unparsed_request->data, old_unparsed_request->data, old_unparsed_request->len);
offset += old_unparsed_request->len;
}
void *buf;

for (chain_link = in; chain_link; chain_link = chain_link->next ) {
ngx_memcpy(ctx->unparsed_request->data + offset, chain_link->buf->pos, chain_link->buf->last - chain_link->buf->pos);
offset += chain_link->buf->last - chain_link->buf->pos;
buf = ngx_array_push_n(&ctx->unparsed_request, chain_link->buf->last - chain_link->buf->pos);
ngx_memcpy(buf, chain_link->buf->pos, chain_link->buf->last - chain_link->buf->pos);
}

ctx->unparsed_request->len = offset;

chain_link = ngx_chain_last_link(in);

return chain_link->buf->last_buf ? NGX_OK: NGX_AGAIN;
Expand Down Expand Up @@ -220,6 +190,7 @@ ngx_http_zip_main_request_header_filter(ngx_http_request_t *r)
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "mod_zip: X-Archive-Files found");

if ((ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_zip_ctx_t))) == NULL
|| ngx_array_init(&ctx->unparsed_request, r->pool, 64 * 1024, 1) == NGX_ERROR
Copy link

Choose a reason for hiding this comment

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

Can't we use Content-Length header to know size of buffer to allocate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might not be available if the input is being streamed.

(Could check if it's available and use it in that case - though not sure it's worth the extra complexity.)

|| ngx_array_init(&ctx->files, r->pool, 1, sizeof(ngx_http_zip_file_t)) == NGX_ERROR
|| ngx_array_init(&ctx->ranges, r->pool, 1, sizeof(ngx_http_zip_range_t)) == NGX_ERROR
|| ngx_array_init(&ctx->pass_srq_headers, r->pool, 1, sizeof(ngx_str_t)) == NGX_ERROR)
Expand Down
2 changes: 1 addition & 1 deletion ngx_http_zip_module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ typedef struct {
} ngx_http_zip_piece_t;

typedef struct {
ngx_str_t *unparsed_request;
ngx_array_t unparsed_request;
ngx_http_zip_piece_t *pieces;
ngx_array_t files;
ngx_array_t ranges;
Expand Down
6 changes: 3 additions & 3 deletions ngx_http_zip_parsers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,9 @@ ngx_int_t
ngx_http_zip_parse_request(ngx_http_zip_ctx_t *ctx)
{
int cs;
u_char *p = ctx->unparsed_request->data;
u_char *pe = ctx->unparsed_request->data + ctx->unparsed_request->len;
u_char *eof = ctx->unparsed_request->data + ctx->unparsed_request->len;
u_char *p = ctx->unparsed_request.elts;
u_char *pe = p + ctx->unparsed_request.nelts;
u_char *eof = pe;
ngx_http_zip_file_t *parsing_file = NULL;


Expand Down
6 changes: 3 additions & 3 deletions ngx_http_zip_parsers.rl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ ngx_int_t
ngx_http_zip_parse_request(ngx_http_zip_ctx_t *ctx)
{
int cs;
u_char *p = ctx->unparsed_request->data;
u_char *pe = ctx->unparsed_request->data + ctx->unparsed_request->len;
u_char *eof = ctx->unparsed_request->data + ctx->unparsed_request->len;
u_char *p = ctx->unparsed_request.elts;
u_char *pe = p + ctx->unparsed_request.nelts;
u_char *eof = pe;
ngx_http_zip_file_t *parsing_file = NULL;

%%{
Expand Down