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.

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

Fix memory usage when reading input #94

Merged
merged 1 commit into from
Dec 17, 2022

Conversation

dimaryaz
Copy link
Contributor

@dimaryaz dimaryaz commented Aug 5, 2022

Currently, every time mod_zip reads 8KB, it allocates a new string, copies all existing data,
and doesn't free the old string. This results in GBs of memory use for a 10MB input.

Just use an nginx array which automatically doubles in size as needed.
(Still leaks memory, but not as much.)

Fixes #67

Currently, every time mod_zip reads 8KB, it allocates a new string, copies all existing data,
and doesn't free the old string. This results in GBs of memory use for a 10MB input.

Just use an nginx array which automatically doubles in size as needed.
(Still leaks memory, but not as much.)

Fixes evanmiller#67
@@ -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

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.)

@evanmiller
Copy link
Owner

Thanks!

@evanmiller evanmiller merged commit b2b1b93 into evanmiller:master Dec 17, 2022
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.

Excessive memory usage
3 participants