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

Caste various regex params to string to avoid thread abort #23

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@p0pr0ck5
Contributor

p0pr0ck5 commented Feb 3, 2016

This commit updates regex.lua to force the use of string types
in situations where the '#' operator is called. This resolves
potential thread abort situations where non-string types are used
as parameters to ngx.re.(g)sub, ngx.re.(g)match and ngx.re.find
as noted in issue #22.

Caste various regex params to string to avoid thread abort
This commit updates regex.lua to force the use of string types
in situations where the '#' operator is called. This resolves
potential thread abort situations where non-string types are used
as parameters to ngx.re.(g)sub, ngx.re.(g)match and ngx.re.find
as noted in issue #22.
@@ -353,6 +353,11 @@ end
local function re_match_helper(subj, regex, opts, ctx, want_caps, res, nth)
-- we need to caste these to a string to avoid a potential thread abort
-- see http://bit.ly/1SH0oz6
regex = tostring(regex)

This comment has been minimized.

@agentzh

agentzh Feb 3, 2016

Member

I think regexes should always be of string type. I cannot think of real use cases of passing numbers or booleans as the pattern. These cases are almost always undesired. Thoughts?

@agentzh

This comment has been minimized.

Member

agentzh commented Feb 3, 2016

@p0pr0ck5 I've applied a modified version of your patch to git master. Basically I think the cast on the regex parameter is superfluous. Let me know if you think otherwise. Thanks a lot!

@agentzh agentzh closed this Feb 3, 2016

@p0pr0ck5

This comment has been minimized.

Contributor

p0pr0ck5 commented Feb 3, 2016

I agree, I can't think of a valid use case. I added it in for the sake of completeness. Agreed that removing it as superfluous is acceptable. Glad it's in, looking forward to this version of resty core getting bundled. Thanks!

@p0pr0ck5 p0pr0ck5 deleted the p0pr0ck5:regex-octothorpe branch Dec 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment