Skip to content

Conversation

@zhuizhuhaomeng
Copy link
Contributor

@zhuizhuhaomeng zhuizhuhaomeng commented Dec 26, 2021

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.

the original PR is #1384.
the ngx.header was rewrite via ffi. So need to rewrite this feature.

values[0].data = p;
values[0].len = ngx_http_time(p, last_modified) - p;
return 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

there should be a blank line before the break;

values[0].data = p;
values[0].len = ngx_http_time(p, last_modified) - p;
return 1;
}
Copy link
Member

Choose a reason for hiding this comment

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

maybe we can return 0 here when the last_modified < 0? just for better performance.

break;

case 13:
last_modified = r->headers_out.last_modified_time;
Copy link
Member

Choose a reason for hiding this comment

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

we should check if the key_buf is Last-Modified-time?




=== TEST 96: Expose the 'Last-Modified' response header as ngx.header["Last-Modified"]
Copy link
Member

Choose a reason for hiding this comment

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

duplicated test name, better use a unique one.
Do you want to test the header is not existing?


case 13:
if (ngx_strncasecmp(key_buf, (u_char *) "Last-Modified", 13) == 0) {

Copy link
Member

Choose a reason for hiding this comment

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

style: useless blank line.

@zhuizhuhaomeng zhuizhuhaomeng merged commit f36fe29 into master Jan 4, 2022
@zhuizhuhaomeng zhuizhuhaomeng deleted the last_modified_header branch January 4, 2022 05:14
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.

5 participants