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

Support compressed ssl sockets #43

Merged
merged 5 commits into from
Jan 18, 2023

Conversation

tw1nk
Copy link
Contributor

@tw1nk tw1nk commented Jan 17, 2023

Avro handles compression over ssl in a quite odd way.

Normally when you do compression and ssl for writes you would first do compression then encryption
Avro does it the other way around, first encryption then compression.

Copy link
Collaborator

@vykulakov vykulakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution but it seems the existing code may be reused in a better way, please check my comments.

transports/sslsocket.go Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
transports/sslsocket.go Outdated Show resolved Hide resolved
transports/sslsocket.go Outdated Show resolved Hide resolved
Refactored transports to implement net.Conn interface, this makes it possible to use
the zlib as an underlying transport for TLS.

re-ordered the client transport stack to have the buffered transport
on top as it would otherwise conflict with the TLS transport

New order of transports (ordered from TCP connection to library exposed)
* TCP socket connection
* Compression (if compression level > 0)
* TLS (if TLSConfig is set)
* Buffer (if BufferSize > 0)
Copy link
Collaborator

@vykulakov vykulakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, now it much better I think. That would be great if you remove commented block of code but in general you may merge it now.

@vykulakov vykulakov merged commit 5e83f8a into myzhan:master Jan 18, 2023
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

Successfully merging this pull request may close these issues.

None yet

2 participants