Add options.buffer in case the TLS handshake has to be read by other piece of code + misc#361
Conversation
Was this left out when creationix was trying to migrate from coro-tls? Is this a planned feature? I have zero clue.
|
I have now added an another commit that allows a table with chunks to be written. This fixes nested TLS connections (on HTTPS proxies, etc), and potentially other cases. |
|
I think this is overall a good change, the cases for it are rare, but they do exist I guess, such as in the case you first read something from a TCP socketing without knowing it is TLS, then attempting to do the TLS handshake. There is also an alternative approach that's more flexible in this situation, why not expose Good catch on the table write, this is what luv and coro-channel writers allow, apparently not lua-openssl though, so adding that compatibility back is a good idea. #343 refactors most of the left-overs and overhauls the overall readability of the code, including this comment. |
|
That might work better, but for me - just reading the client hello is enough, zero writing. This was probably the most simple approach. I might implement it if it's easy enough, and I see benefits of having stronger controls like this. |
|
Well, more specifically the write would have to be to the openssl bin buffer, not to the SSL object I am not sure if that's exposed in the SSL object though so a change like this might involve exposing more stuff.
Another note for the table write, why not use |
|
Yeah, onCipher could be used in theory. (once it gets added - not until now afaik) Using ipairs sounds like a great idea. Changing to it now. |
deps/secure-socket/biowrap.lua
Outdated
| function ssocket.write(_, plain, callback) | ||
| if type(plain) == "table" then | ||
| for i=1, #plain do | ||
| for i in ipairs(plain) do |
There was a problem hiding this comment.
that depends on __index behavior, instead use the value returned by ipairs:
for _, chunk in ipairs(plain) do
ssl:write(chunk)
endForce pushing is bad.
Bilal2453
left a comment
There was a problem hiding this comment.
Looks good enough to me
This is pretty much a "me" feature. Feel free to drop it.
Also, I removed the note about writeCipher. Was this left out when creationix was trying to migrate from coro-tls? Is this a planned feature? I have zero clue. It shouldn't be here as of now, I think.