[Feature] Add ICAP Content-Type and Filename from TODO List #4595
Conversation
| local in_fname = p:get_filename() | ||
| local in_type, in_stype = p:get_detected_type() | ||
| local part_info = {in_fname,in_type,in_stype} | ||
| cfg.check(task, content, p:get_digest(), rule, part_info) |
There was a problem hiding this comment.
I think the original idea was just to pass mime_part object (hence p) and not to split it into some array.
There was a problem hiding this comment.
You are absolutely right. How I couldn't think of that... I found that maybe_part is always nil, but it didn't occur to me to just pass p object instead of doing all these variable assignments here.
However, will that mean that the extraction of content type and subtype, as well as filename should go in icap.lua in lua_scanners?
| local in_type = "application" | ||
| local in_stype = "octet-stream" | ||
| local part_info = {in_fname,in_type,in_stype} | ||
| cfg.check(task, task:get_content(), task:get_digest(), rule, part_info) |
There was a problem hiding this comment.
If we don't have part, then we must just pass nil here and reconstruct the necessary information afterwards.
There was a problem hiding this comment.
Maybe if it's nil we can stick with the already available req_fake_url? Otherwise, I am not sure how I can reconstruct the missing information in the lua_scanner itself... May you can point me in the right direction?
lbahtarliev
force-pushed
the
master
branch
2 times, most recently
from
dab75e6
to
1d9498a
Compare
lualib/lua_scanners/icap.lua
Outdated
| @@ -240,10 +240,19 @@ local function icap_check(task, content, digest, rule, maybe_part) | |||
|
|
|||
| local function get_req_headers() | |||
|
|
|||
| local in_client_ip = task:get_from_ip() | |||
| lua_util.debugm(rule.name, task, 'URL: http://%s/%s | Content-Type: %s/%s', | |||
There was a problem hiding this comment.
This will crash the plugin when maybe_part is nil. Lua does not do lazy evaluations. Also, that will call two C functions twice (get_filename and get_detected_type).
There was a problem hiding this comment.
Didn't notice I am missing a check before the debug message. Is it OK to just remove it?
There was a problem hiding this comment.
Just removed the debug message
There was a problem hiding this comment.
It looks good to me, thank you!
… changes. Remove unnecessary debug message.
It turns out, that external_services plugin is not submitting maybe_part parameter at all when calling icap services check method. In order to avoid expensive operations in icap.lua scanner, I am just getting the filename and content-type/subtype in the external_services plugin and submitting that as maybe_part parameter. Of course, a new parameter can be introduced should maybe_part is used, but I wasn't able to find any usage of it at least related to the ICAP functionality.