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

feat(frame/headers): Packaging headers pseudo order type #8

Merged
merged 4 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 1 addition & 50 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,44 +22,6 @@ jobs:

- run: cargo fmt --all --check

test:
name: Test
needs: [style]
runs-on: ubuntu-latest
env:
RUSTFLAGS: -Dwarnings
strategy:
matrix:
rust:
- beta
- stable
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Install Rust (${{ matrix.rust }})
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust }}

- name: Install libssl-dev
run: sudo apt-get update && sudo apt-get install libssl-dev
- name: Build without unstable flag
run: cargo build

- name: Check with unstable flag
run: cargo check --features unstable

- name: Run lib tests and doc tests
run: cargo test

- name: Run integration tests
run: cargo test -p h2-tests

- name: Run h2spec
run: ./ci/h2spec.sh
if: matrix.rust == 'stable'

#clippy_check:
# runs-on: ubuntu-latest
# steps:
Expand Down Expand Up @@ -92,15 +54,4 @@ jobs:
cargo update
cargo update --package tokio --precise 1.38.1

- run: cargo check -p h2

minimal-versions:
runs-on: ubuntu-latest
needs: [style]
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
- uses: dtolnay/rust-toolchain@stable
- uses: taiki-e/install-action@cargo-hack
- uses: taiki-e/install-action@cargo-minimal-versions
- run: cargo minimal-versions --ignore-private check
- run: cargo check -p rh2
9 changes: 5 additions & 4 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@
use crate::codec::{Codec, SendError, UserError};
use crate::ext::Protocol;
use crate::frame::{
Headers, Pseudo, PseudoOrder, Reason, Settings, SettingsOrder, StreamDependency, StreamId,
Headers, Pseudo, PseudoOrder, PseudoOrders, Reason, Settings, SettingsOrder, StreamDependency,
StreamId,
};
use crate::proto::{self, Error};
use crate::{FlowControl, PingPong, RecvStream, SendStream};
Expand Down Expand Up @@ -347,7 +348,7 @@ pub struct Builder {
local_max_error_reset_streams: Option<usize>,

/// The headers frame pseudo order
headers_pseudo_order: Option<[PseudoOrder; 4]>,
headers_pseudo_order: Option<PseudoOrders>,

/// The headers frame priority
headers_priority: Option<StreamDependency>,
Expand Down Expand Up @@ -678,7 +679,7 @@ impl Builder {

/// Set http2 header pseudo order
pub fn headers_psuedo(&mut self, headers_psuedo: [PseudoOrder; 4]) -> &mut Self {
self.headers_pseudo_order = Some(headers_psuedo);
self.headers_pseudo_order = Some(headers_psuedo.into());
self
}

Expand Down Expand Up @@ -1614,7 +1615,7 @@ impl Peer {
request: Request<()>,
protocol: Option<Protocol>,
end_of_stream: bool,
pseudo_order: Option<[PseudoOrder; 4]>,
pseudo_order: Option<PseudoOrders>,
headers_priority: Option<StreamDependency>,
) -> Result<Headers, SendError> {
use http::request::Parts;
Expand Down
80 changes: 41 additions & 39 deletions src/frame/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pub struct Pseudo {
pub status: Option<StatusCode>,

// order of pseudo headers
pub order: Option<[PseudoOrder; 4]>,
pub order: PseudoOrders,
}

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand All @@ -84,6 +84,26 @@ pub enum PseudoOrder {
Path,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct PseudoOrders([PseudoOrder; 4]);

impl From<[PseudoOrder; 4]> for PseudoOrders {
fn from(src: [PseudoOrder; 4]) -> Self {
PseudoOrders(src)
}
}

impl Default for PseudoOrders {
fn default() -> Self {
PseudoOrders([
PseudoOrder::Method,
PseudoOrder::Scheme,
PseudoOrder::Authority,
PseudoOrder::Path,
])
}
}

#[derive(Debug)]
pub struct Iter {
/// Pseudo headers
Expand Down Expand Up @@ -582,7 +602,7 @@ impl Pseudo {
method: Method,
uri: Uri,
protocol: Option<Protocol>,
pseudo_order: Option<[PseudoOrder; 4]>,
order: Option<PseudoOrders>,
) -> Self {
let parts = uri::Parts::from(uri);

Expand Down Expand Up @@ -612,7 +632,7 @@ impl Pseudo {
path,
protocol,
status: None,
order: pseudo_order,
order: order.unwrap_or_default(),
};

// If the URI includes a scheme component, add it to the pseudo headers
Expand All @@ -637,7 +657,7 @@ impl Pseudo {
path: None,
protocol: None,
status: Some(status),
order: None,
order: Default::default(),
}
}

Expand Down Expand Up @@ -732,47 +752,29 @@ impl Iterator for Iter {
use crate::hpack::Header::*;

if let Some(ref mut pseudo) = self.pseudo {
if let Some(orders) = pseudo.order.as_ref() {
for pseudo_type in orders {
match pseudo_type {
PseudoOrder::Method => {
if let Some(method) = pseudo.method.take() {
return Some(Method(method));
}
for pseudo_type in pseudo.order.0.iter() {
match pseudo_type {
PseudoOrder::Method => {
if let Some(method) = pseudo.method.take() {
return Some(Method(method));
}
PseudoOrder::Scheme => {
if let Some(scheme) = pseudo.scheme.take() {
return Some(Scheme(scheme));
}
}
PseudoOrder::Scheme => {
if let Some(scheme) = pseudo.scheme.take() {
return Some(Scheme(scheme));
}
PseudoOrder::Authority => {
if let Some(authority) = pseudo.authority.take() {
return Some(Authority(authority));
}
}
PseudoOrder::Authority => {
if let Some(authority) = pseudo.authority.take() {
return Some(Authority(authority));
}
PseudoOrder::Path => {
if let Some(path) = pseudo.path.take() {
return Some(Path(path));
}
}
PseudoOrder::Path => {
if let Some(path) = pseudo.path.take() {
return Some(Path(path));
}
}
}
} else {
if let Some(method) = pseudo.method.take() {
return Some(Method(method));
}

if let Some(scheme) = pseudo.scheme.take() {
return Some(Scheme(scheme));
}

if let Some(authority) = pseudo.authority.take() {
return Some(Authority(authority));
}

if let Some(path) = pseudo.path.take() {
return Some(Path(path));
}
}

if let Some(protocol) = pseudo.protocol.take() {
Expand Down
3 changes: 2 additions & 1 deletion src/frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ pub use self::data::Data;
pub use self::go_away::GoAway;
pub use self::head::{Head, Kind};
pub use self::headers::{
parse_u64, Continuation, Headers, Pseudo, PseudoOrder, PushPromise, PushPromiseHeaderError,
parse_u64, Continuation, Headers, Pseudo, PseudoOrder, PseudoOrders, PushPromise,
PushPromiseHeaderError,
};
pub use self::ping::Ping;
pub use self::priority::{Priority, StreamDependency};
Expand Down
4 changes: 2 additions & 2 deletions src/proto/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::frame::DEFAULT_INITIAL_WINDOW_SIZE;
use crate::proto::*;

use bytes::Bytes;
use frame::{PseudoOrder, StreamDependency};
use frame::{PseudoOrders, StreamDependency};
use futures_core::Stream;
use std::io;
use std::marker::PhantomData;
Expand Down Expand Up @@ -84,7 +84,7 @@ pub(crate) struct Config {
pub remote_reset_stream_max: usize,
pub local_error_reset_streams_max: Option<usize>,
pub settings: frame::Settings,
pub headers_pseudo_order: Option<[PseudoOrder; 4]>,
pub headers_pseudo_order: Option<PseudoOrders>,
pub headers_priority: Option<StreamDependency>,
}

Expand Down
10 changes: 5 additions & 5 deletions src/proto/streams/streams.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::frame::{PseudoOrder, StreamDependency};
use super::frame::{PseudoOrders, StreamDependency};
use super::recv::RecvHeaderBlockError;
use super::store::{self, Entry, Resolve, Store};
use super::{Buffer, Config, Counts, Prioritized, Recv, Send, Stream, StreamId};
Expand Down Expand Up @@ -36,7 +36,7 @@ where
send_buffer: Arc<SendBuffer<B>>,

/// Headers frame pseudo order
headers_pseudo_order: Option<[PseudoOrder; 4]>,
headers_pseudo_order: Option<PseudoOrders>,

/// Headers frame priority
headers_priority: Option<StreamDependency>,
Expand Down Expand Up @@ -118,7 +118,7 @@ where
pub fn new(
config: Config,
headers_priority: Option<StreamDependency>,
headers_pseudo_order: Option<[PseudoOrder; 4]>,
headers_pseudo_order: Option<PseudoOrders>,
) -> Self {
let peer = P::r#dyn();

Expand Down Expand Up @@ -1062,8 +1062,8 @@ where
Streams {
inner: self.inner.clone(),
send_buffer: self.send_buffer.clone(),
headers_priority: self.headers_priority.clone(),
headers_pseudo_order: self.headers_pseudo_order.clone(),
headers_priority: self.headers_priority,
headers_pseudo_order: self.headers_pseudo_order,
_p: ::std::marker::PhantomData,
}
}
Expand Down