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

proxy_pass + "Content-Type" again. #1456

Closed
ecc256 opened this issue Jan 15, 2019 · 12 comments
Closed

proxy_pass + "Content-Type" again. #1456

ecc256 opened this issue Jan 15, 2019 · 12 comments

Comments

@ecc256
Copy link

ecc256 commented Jan 15, 2019

Guys,
Want to bring up old question.
It’s not a bug, I’m just trying to understand behavior and find right course of actions.
I did lots of Googling, but didn’t find definitive answer.
(Openresty version is at the very bottom of the post.)

In short:
Static files are stored on Azure storage.
There is a "location" configured for such files which performs uri rewrite, adds sas token to and does
proxy_pass http://qamfiles.file.core.windows.net;

It works fine.
The only wrinkle is "Content-Type" returned by proxy (i.e. Azure storage http endpoint) is "application/octet-stream" always.
Requested file extension is known and proper "Content-Type" can be set easily from header_filter_by_lua_file code.

Question:
Is there a better way to tell OpenResty to update "Content-Type" header according to the file type after proxying?

If not, I assume, "mime.types" file is loaded somewhere into memory.
Is it possible to access it (as a table?) from LUA code directly instead of essentially duplicating its content (in LUA code)?

lua_use_default_type setting for the location has no effect and is right behavior, apparently.

Not sure if it’s related to 1449
Thanks!

I didn’t provide config example, I it’s not needed for this case.

[root@nginx ~]# /usr/local/openresty/nginx/sbin/nginx -V
nginx version: openresty/1.13.6.2
built by gcc 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC) 
built with OpenSSL 1.1.0h  27 Mar 2018
TLS SNI support enabled
configure arguments: --prefix=/usr/local/openresty/nginx --with-cc-opt='-O2 -DNGX_LUA_ABORT_AT_PANIC -I/usr/local/openresty/zlib/include -I/usr/local/openresty/pcre/include -I/usr/local/openresty/openssl/include' --add-module=../ngx_devel_kit-0.3.0 --add-module=../echo-nginx-module-0.61 --add-module=../xss-nginx-module-0.06 --add-module=../ngx_coolkit-0.2rc3 --add-module=../set-misc-nginx-module-0.32 --add-module=../form-input-nginx-module-0.12 --add-module=../encrypted-session-nginx-module-0.08 --add-module=../srcache-nginx-module-0.31 --add-module=../ngx_lua-0.10.13 --add-module=../ngx_lua_upstream-0.07 --add-module=../headers-more-nginx-module-0.33 --add-module=../array-var-nginx-module-0.05 --add-module=../memc-nginx-module-0.19 --add-module=../redis2-nginx-module-0.15 --add-module=../redis-nginx-module-0.3.7 --add-module=../ngx_stream_lua-0.0.5 --with-ld-opt='-Wl,-rpath,/usr/local/openresty/luajit/lib -L/usr/local/openresty/zlib/lib -L/usr/local/openresty/pcre/lib -L/usr/local/openresty/openssl/lib -Wl,-rpath,/usr/local/openresty/zlib/lib:/usr/local/openresty/pcre/lib:/usr/local/openresty/openssl/lib' --with-pcre-jit --with-stream --with-stream_ssl_module --with-stream_ssl_preread_module --with-http_v2_module --without-mail_pop3_module --without-mail_imap_module --without-mail_smtp_module --with-http_stub_status_module --with-http_realip_module --with-http_addition_module --with-http_auth_request_module --with-http_secure_link_module --with-http_random_index_module --with-http_gzip_static_module --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gunzip_module --with-threads --with-dtrace-probes --with-stream --with-stream_ssl_module --with-http_ssl_module
@agentzh
Copy link
Member

agentzh commented Feb 4, 2019

@ecc256 Recently we fixed bugs in this part. Will you please try the following preview version of OpenResty on your side? If you still have problems, please let us know by following up here. Sorry for the delay on our side.

https://openresty.org/download/openresty-1.15.8.1rc0.tar.gz

@ecc256
Copy link
Author

ecc256 commented Feb 4, 2019

@agentzh,

Sorry for the delay on our side.

No problem at all!
Just tried openresty-1.15.8.1rc0

I don’t see any changes for my case.
Just to make sure, we are on the same page:
My setup gist. (comments are appreciated)

# /usr/local/openresty/nginx/sbin/nginx -V
nginx version: openresty/1.15.8.1rc0
built by gcc 4.8.5 20150623 (Red Hat 4.8.5-36) (GCC) 
built with OpenSSL 1.0.2k-fips  26 Jan 2017
TLS SNI support enabled
configure arguments: --prefix=/usr/local/openresty/nginx --with-cc-opt=-O2 --add-module=../ngx_devel_kit-0.3.1rc1 --add-module=../echo-nginx-module-0.61 --add-module=../xss-nginx-module-0.06 --add-module=../ngx_coolkit-0.2 --add-module=../set-misc-nginx-module-0.32 --add-module=../form-input-nginx-module-0.12 --add-module=../encrypted-session-nginx-module-0.08 --add-module=../srcache-nginx-module-0.31 --add-module=../ngx_lua-0.10.14rc5 --add-module=../ngx_lua_upstream-0.07 --add-module=../headers-more-nginx-module-0.33 --add-module=../array-var-nginx-module-0.05 --add-module=../memc-nginx-module-0.19 --add-module=../redis2-nginx-module-0.15 --add-module=../redis-nginx-module-0.3.7 --add-module=../rds-json-nginx-module-0.15 --add-module=../rds-csv-nginx-module-0.09 --add-module=../ngx_stream_lua-0.0.6rc4 --with-ld-opt=-Wl,-rpath,/usr/local/openresty/luajit/lib --with-stream --with-stream_ssl_module --with-stream_ssl_preread_module --with-http_ssl_module

Scenario:
upstream server set "Content-Type" to "application/octet-stream" always.
I have to set to proper "Content-Type", based on file extension in header_filter_by_lua_file.

If I do
	ngx.header.content_type = nil
in
	header_filter_by_lua_file
then
	client won't get "Content-Type" header at all.

Question:

  1. What condition should be met so OpenResty set "Content-Type" based on file extension after proxy-ing?
  2. I guess, header_filter_by_lua_file is fired too late for 编译错误 #1 to work, right?
  3. If 编译错误 #1 if not possible in my scenario, is there a way to access (already loaded to memory) "mime.types" file from LUA code directly, instead of duplicating its content in my header_filter_by_lua_file.

BTW:
I think, the current OpenResty behavior is correct.
It just doesn't suit my requirements.
Basically, I'm looking for good solution or advice for question #3 only.

Let me know if further clarification or tests are needed, please.
Thanks!

@agentzh
Copy link
Member

agentzh commented Feb 4, 2019

@ecc256 OK, I see your requirement now. I've just prepared and tested the following minimal and standalone example, which should serve you well:

http {
    init_by_lua_block {
        require "resty.core"

        require "ffi".cdef[[
            uintptr_t ngx_http_set_content_type(void *r)
        ]]
    }

    server {
        listen 8080;

        location ~ '^/t/(.*)' {
            types {
                text/html   html htm shtml;
                text/css    css;
                text/xml    xml;
                image/gif   gif;
            }

             proxy_pass $scheme://127.0.0.1:$server_port/$1;

             header_filter_by_lua_block {
                ngx.header["content-type"] = nil
                local get_req = require "resty.core.base".get_request
                require "ffi".C.ngx_http_set_content_type(get_req())
             }
        }

        location /fake-backend/ {
            content_by_lua_block {
                ngx.header["Content-Type"] = 'application/octet-stream';
                ngx.say("ok")
            }
        }
    }
}

And then we query the proxy locations with different file extensions:

$ curl -i localhost:8080/t/fake-backend/a.html
HTTP/1.1 200 OK
Server: nginx/1.15.8 (no pool)
Date: Mon, 04 Feb 2019 23:09:42 GMT
Content-Type: text/html
Content-Length: 3
Connection: keep-alive

ok

$ curl -i localhost:8080/t/fake-backend/a.css
HTTP/1.1 200 OK
Server: nginx/1.15.8 (no pool)
Date: Mon, 04 Feb 2019 23:09:48 GMT
Content-Type: text/css
Content-Length: 3
Connection: keep-alive

ok

The Content-Type is indeed varying. And we can try accessing the fake backend location directly:

$ curl -i localhost:8080/fake-backend/a.html
HTTP/1.1 200 OK
Server: nginx/1.15.8 (no pool)
Date: Mon, 04 Feb 2019 23:14:10 GMT
Content-Type: application/octet-stream
Transfer-Encoding: chunked
Connection: keep-alive

ok

$ curl -i localhost:8080/fake-backend/a.css
HTTP/1.1 200 OK
Server: nginx/1.15.8 (no pool)
Date: Mon, 04 Feb 2019 23:14:28 GMT
Content-Type: application/octet-stream
Transfer-Encoding: chunked
Connection: keep-alive

ok

It's indeed always application/octet-stream, just like your Azure storage's API.

Yeah, it uses a hack to achieve this. It would be better to prepare a Lua library to encapsulate this hack :)

@ecc256
Copy link
Author

ecc256 commented Feb 5, 2019

@agentzh,
Super!

Yeah, it uses a hack to achieve this. It would be better to prepare a Lua library to encapsulate this hack :)

I do disagree respectfully!
It does show how to use OpenResty low level API (call exported C function from LUA)
There are plenty examples in /usr/local/openresty/lualib/, but none of them is concise and easy to understand as the one you showed.

I use it in few locations, so I’ve rewrote your code with header_filter_by_lua_file as:

local base = require "resty.core.base"

local ffi = require "ffi"
local C = ffi.C

ffi.cdef[[
	uintptr_t ngx_http_set_content_type(void *r)
]]

local get_request = base.get_request

ngx.header["content-type"] = nil
C.ngx_http_set_content_type(get_request())

It works great!

Tiny question (unrelated to the issue):

You use
ngx.header["content-type"] = nil
not
ngx.header.content_type = nil
Is it because the latter variant needs to extract variable name and call the former "under the hood", right?

Last thing (unrelated to the issue):
stub_status statement doesn’t work anymore in 1.15.8.1rc0.
Was it deprecated or replaced to something else?
Is there a full list of changes somewhere?

And thanks a lot for your help and for great example!

BTW: Should it be:
proxy_pass $scheme://10.12.64.7:$server_port/fake-backend/$1;
instead of:
proxy_pass $scheme://127.0.0.1:$server_port/$1;
?

@agentzh
Copy link
Member

agentzh commented Feb 5, 2019

@ecc256 No, do not call ffi.cdef() in a request handler directly since it is VERY SLOW and also leaks memory. Call it in init_by_lua* or in a Lua module scope instead.

@agentzh
Copy link
Member

agentzh commented Feb 5, 2019

@ecc256 Calling ffi.cdef() every time a request is served is the last thing you would want. Make sure you only do that once for the whole nginx server instance.

@agentzh
Copy link
Member

agentzh commented Feb 5, 2019

There is one missing thing in my example, BTW. We should check the return value of the C.ngx_http_set_content_type(get_request()) call is 0 (or NGX_OK on the C land). This call may fail, though not likely.

@agentzh agentzh closed this as completed Feb 5, 2019
@ecc256
Copy link
Author

ecc256 commented Feb 5, 2019

@agentzh,
Got it!
I’ll move

local ffi = require "ffi"
ffi.cdef[[
	uintptr_t ngx_http_set_content_type(void *r)
]]

into init_worker_by_lua_file!
Yeah, I’m a pretty much newbie still...

We should check the return value of the C.ngx_http_set_content_type(get_request()) call is 0 (or NGX_OK on the C land). This call may fail, though not likely.

Even if it fails, what would be the right thing to do?
Set
ngx.header["content-type"] = "application/octet-stream"
again?

Thanks for the lesson! 👍

@agentzh
Copy link
Member

agentzh commented Feb 5, 2019

You use
ngx.header["content-type"] = nil
not
ngx.header.content_type = nil
Is it because the latter variant needs to extract variable name and call the former "under the hood", right?

The former form is a little bit faster.

It does show how to use OpenResty low level API (call exported C function from LUA)

Well, that is actually NGINX's C API. Because NGINX does not have the concept of an ABI, it may break in a future nginx release. Fortunately for this particular C API function, it hasn't changed for years. Beware though.

stub_status statement doesn’t work anymore in 1.15.8.1rc0.

Well, you just need to enable this module by passing --with-http_stub_status_module to the ./configure command line when building OpenResty from source. This is the same for the standard nginx releases. This is not something new. It's always like that.

BTW: Should it be:
proxy_pass $scheme://10.12.64.7:$server_port/fake-backend/$1;
instead of:
proxy_pass $scheme://127.0.0.1:$server_port/$1;
?

No, this example wants to proxy to location /fake-backend/, otherwise you'll probably get 404 when testing this example.

@agentzh
Copy link
Member

agentzh commented Feb 5, 2019

I’ll move

local ffi = require "ffi"
ffi.cdef[[
	uintptr_t ngx_http_set_content_type(void *r)
]]

into init_worker_by_lua_file!

Well, init_worker_by_lua_file is a bit slower than init_by_lua* since every nginx worker process has to do it once upon spawning. Also, the latter has a chance to enjoy the operating system's COW optimization and save a little bit of memory.

@ecc256
Copy link
Author

ecc256 commented Feb 5, 2019

Well, init_worker_by_lua_file is a bit slower than init_by_lua* since every nginx worker process has to do it once upon spawning. Also, the latter has a chance to enjoy the operating system's COW optimization and save a little bit of memory.

I see.
Still much to learn...!

Well, you just need to enable this module by passing --with-http_stub_status_module to the ./configure command line when building OpenResty from source. This is the same for the standard nginx releases. This is not something new. It's always like that.

Got it.
I followed CentOS build here.
There is no mention of --with-http_stub_status_module there.
Not a big deal I assume!

Well, that is actually NGINX's C API. Because NGINX does not have the concept of an ABI, it may break in a future nginx release. Fortunately for this particular C API function, it hasn't changed for years. Beware though.

Got it!

No, this example wants to proxy to location /fake-backend/, otherwise you'll get 404 when testing this example.

I’m a bit confused about right proxy_pass statement still…

To clarify, you comment means the correct line is:
proxy_pass $scheme://10.12.64.7:$server_port/fake-backend/$1;

proxy_pass $scheme://127.0.0.1:$server_port/$1;
returns 404, since there is no location / { ... } defined, right?
Sorry to keep bugging you…

@agentzh
Copy link
Member

agentzh commented Feb 5, 2019

@ecc256 That proxy_pass line is only for my standalone example. It's not meant to be used elsewhere like in your production config.

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

No branches or pull requests

2 participants