torhve / weechat-matrix-protocol-script

A WeeChat script in Lua that implements the matrix.org chat protocol
349 stars 52 forks source link

Fixed HTTP/2 requests being classed as invalid #129

Closed astrakk closed 5 years ago

astrakk commented 5 years ago

Simply changes the regex used to parse http status lines to also match "HTTP/2".

This commit fixes #128

torhve commented 5 years ago

Thanks!

foxcpp commented 5 years ago
"^HTTP/(1%.[01]) (%d%d%d) (.-)\r?\n"

Old pattern matches HTTP/1.0 and HTTP/1.1.

"^HTTP/([012]) (%d%d%d) (.-)\r?\n"

New pattern matches HTTP/0 (!?), HTTP/1 and HTTP/2.

Wouldn't it cause problems if server returns HTTP/1.0 or HTTP/1.1?

astrakk commented 5 years ago
"^HTTP/(1%.[01]) (%d%d%d) (.-)\r?\n"

Old pattern matches HTTP/1.0 and HTTP/1.1.

"^HTTP/([012]) (%d%d%d) (.-)\r?\n"

New pattern matches HTTP/0 (!?), HTTP/1 and HTTP/2.

Wouldn't it cause problems if server returns HTTP/1.0 or HTTP/1.1?

I think you might be right! I've tried fixing it up properly but can't quite wrap my head around how to do it using Lua Patterns.

The regex equivalent I worked out is ^HTTP/(1\.[01]|[12])

foxcpp commented 5 years ago

UPD: Oops, I haven't noticed your new PR. Ok, that would work too.

According to http://lua-users.org/wiki/PatternsTutorial:

Limitations of Lua patterns

Especially if you're used to other languages with regular expressions, you might expect to be able to do >stuff like this:

'(foo)+' -- match the string "foo" repeated one or more times '(foo|bar)' -- match either the string "foo" or the string "bar"

Unfortunately Lua patterns do not support this, only single characters can be repeated or chosen >between, not sub-patterns or strings.

The closest Lua pattern we can get also matches several invalid version strings (last 2).

> string.match('HTTP/2', 'HTTP/[12]%.?[01]?')
HTTP/2
> string.match('HTTP/1.0', 'HTTP/[12]%.?[01]?')
HTTP/1.0
> string.match('HTTP/1', 'HTTP/[12]%.?[01]?')
HTTP/1
> string.match('HTTP/1.1', 'HTTP/[12]%.?[01]?')
HTTP/1.1
> string.match('HTTP/1.', 'HTTP/[12]%.?[01]?')
HTTP/1.
> string.match('HTTP/11', 'HTTP/[12]%.?[01]?')
HTTP/11

One possible solution is to use HTTP/([1-9%.]+) and check version manually.