-
Notifications
You must be signed in to change notification settings - Fork 120
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
[WASM-4] Clean up mtsender, ws support and small changes to make it usable #301
Conversation
This is great! I have previously experimented with making mtsender work with traits for transport, for it to be possible to provide your own custom networking, and to make it pluggable - but it was more of an experimental branch than a real implementation. This change here would significantly simplify the work needed to abstract the transport away behind a trait. Great stuff! |
#[cfg(all(not(all(target_arch = "wasm32", target_os = "unknown")),))] | ||
let addr = { | ||
#[cfg(not(feature = "proxy"))] | ||
let addr = ServerAddr::Tcp { address: tcp_addr }; | ||
|
||
#[cfg(feature = "proxy")] | ||
let addr = if let Some(proxy) = &config.params.proxy_url { | ||
ServerAddr::Proxied { | ||
address: tcp_addr, | ||
proxy: proxy.to_owned(), | ||
} | ||
} else { | ||
ServerAddr::Tcp { address: tcp_addr } | ||
}; | ||
|
||
addr | ||
}; | ||
|
||
#[cfg(all(target_arch = "wasm32", target_os = "unknown"))] | ||
let addr = ServerAddr::Ws { | ||
address: WS_ADDRESSES[dc_id as usize].to_string(), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: can't a WS connection be proxied too?
So far, it looks like we should have something like TcpTransport
. WebSocketTransport
and Proxied<T: Transport>
and enum AnyTransport
with all the supported variants to cover all the permutations properly without creating too much mess.
enum AnyTransport {
Tcp(TcpTransport),
WebSocket(WebSocketTransport),
ProxiedTcp(Proxied<TcpTransport>),
ProxiedWebSocket(Proxied<WebSocketTransport>),
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't a WS connection be proxied too?
No, we're in a browser. We don't have access to connection properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the only use-case for ws is the browser? I can see it used outside of the browser too, in which case it would allow proxying at our end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of this PR is only browser support as I don't have any use for ws in platforms other than in a browser.
With this, we can finally compile grammers for `wasm32-unknown-unknown`.
Telegram only allows connections with transport obfuscation when using ws.
After this PR users should be able to use grammers in the browser! We still have a lot of testing and documenting to do but that's for another PR.
Changes:
NetStream
into a new mod callednet
and separate net code from the actual mtsender implementation.ws_stream_wasm
.js
feature ofgetrandom
to avoid getting compile errors.fs
orproxy
features but is targeting web.