salvo-rs / salvo

A powerful web framework built with a simplified design.
https://salvo.rs
Apache License 2.0
3.4k stars 208 forks source link

Additionally, even when I disable the `http2` feature, the server still handles HTTP/2 requests. I don't think it should behave this way. #853

Closed TheAwiteb closed 3 months ago

TheAwiteb commented 3 months ago
          Additionally, even when I disable the `http2` feature, the server still handles HTTP/2 requests. I don't think it should behave this way.

Originally posted by @TheAwiteb in https://github.com/salvo-rs/salvo/issues/851#issuecomment-2274456183

chrislearn commented 3 months ago

But have you enabled other features such as rustls openssl? These features may cause http2 to be automatically enabled.

rustls = ["http1", "http2", "dep:tokio-rustls", "dep:rustls-pemfile", "ring"]
native-tls = ["http1", "http2", "dep:tokio-native-tls", "dep:native-tls"]
openssl = ["http2", "dep:openssl", "dep:tokio-openssl"]
TheAwiteb commented 3 months ago

These features may cause http2 to be automatically enabled.

Yes I see, do you think changing this is better? because maybe I want my server only support one protocol.

We already emit a compile error if no one of those are enabled

https://github.com/salvo-rs/salvo/blob/affa8cca48cb98aedeec6767f02373bc1ffa4c5e/crates/core/src/server.rs#L7-L10

chrislearn commented 3 months ago

If all these protocols are disabled, what services are you providing?

TheAwiteb commented 3 months ago

If all these protocols are disabled, what services are you providing?

Not all, I only want http2 or only http3, or both together. Do you understand me? Meaning I don't want to support http1 for example, now the server owner is forced to support http1 while they doesn't want to.

This is a breaking change, so we can implement it in the stable release.

chrislearn commented 3 months ago

Your words confuse me. The code here means that as long as you have a feature enabled, there will be no compilation error.

 #[cfg(not(any(feature = "http1", feature = "http2", feature = "quinn")))] 
 compile_error!( 
     "You have enabled `server` feature, it requires at least one of the following features: http1, http2, quinn." 
 ); 
TheAwiteb commented 3 months ago

I mean there is no fear of not enabling one of them. Now if I want to support only http2 I can't and I want to.

chrislearn commented 3 months ago

Why not? Please provide the code you have a problem with.

TheAwiteb commented 3 months ago

I have no problem with the issue, and there is no reason why I would not want to support a particular protocol. But surely there is someone who wants to support only http1 or only http2. If you do not see this as the case, you can close the issue.

chrislearn commented 3 months ago

What I mean is that you only need to make sure that one feature is turned on, and there will be no compilation errors. Compilation errors will only be reported when any protocol is not supported.

TheAwiteb commented 3 months ago

There is a misunderstanding here. Let's say I want my server to support http1 only, and I want rustls to support https. Can I do this now?

From what I see and what I have tried, no, because the rustls feature enables the http2 feature, do you understand me now?