You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by "maskit (via GitHub)" <gi...@apache.org> on 2023/02/16 18:59:26 UTC
[GitHub] [trafficserver] maskit opened a new issue, #9428: Prohibit a second SETTINGS frame
maskit opened a new issue, #9428:
URL: https://github.com/apache/trafficserver/issues/9428
We have `max_settings_per_frame`, `max_settings_per_minute`, and `max_settings_frames_per_minute` for H2, but sending multiple SETTINGS frame on H3 is not allowed.
> SETTINGS frames always apply to an entire HTTP/3 connection, never a single stream. A SETTINGS frame MUST be sent as the first frame of each [control stream](https://datatracker.ietf.org/doc/html/rfc9114#control-streams) (see [Section 6.2.1](https://datatracker.ietf.org/doc/html/rfc9114#control-streams)) by each peer, and it MUST NOT be sent subsequently. If an endpoint receives a second SETTINGS frame on the [control stream](https://datatracker.ietf.org/doc/html/rfc9114#control-streams), the endpoint MUST respond with a [connection error](https://datatracker.ietf.org/doc/html/rfc9114#errors) of type [H3_FRAME_UNEXPECTED](https://datatracker.ietf.org/doc/html/rfc9114#H3_FRAME_UNEXPECTED).[ΒΆ](https://datatracker.ietf.org/doc/html/rfc9114#section-7.2.4-2)
https://datatracker.ietf.org/doc/html/rfc9114#name-settings
It's testable through `H3FrameDispatcher` like the unit test does (Need to add Http3SettingsHandler instead of the mock handler).
https://github.com/apache/trafficserver/blob/2402444b5864411f2415ae142ba81d7637656531/proxy/http3/test/test_Http3FrameDispatcher.cc#L31-L47
It's also testable by calling `Http3SettingsHandler::handleFrame` directly.
https://github.com/apache/trafficserver/blob/2402444b5864411f2415ae142ba81d7637656531/proxy/http3/Http3App.h#L99
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] duke8253 commented on issue #9428: Prohibit a second SETTINGS frame
Posted by "duke8253 (via GitHub)" <gi...@apache.org>.
duke8253 commented on issue #9428:
URL: https://github.com/apache/trafficserver/issues/9428#issuecomment-1593609025
While reading our code and the documentation, I noticed that we don't seem to enforce one control stream per connection?
https://datatracker.ietf.org/doc/html/rfc9114#control-streams
> Only one control stream per peer is permitted; receipt of a second stream claiming to be a control stream MUST be treated as a [connection error](https://datatracker.ietf.org/doc/html/rfc9114#errors) of type [H3_STREAM_CREATION_ERROR](https://datatracker.ietf.org/doc/html/rfc9114#H3_STREAM_CREATION_ERROR).
Also maybe we should rename `_control_stream_dispatcher` to something else, since use it to handle `PUSH` streams too?
https://github.com/apache/trafficserver/blob/a29574d2fd089b90f1b909aedbb67b8000f01711/proxy/http3/Http3App.cc#L211-L218
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] duke8253 commented on issue #9428: Prohibit a second SETTINGS frame
Posted by "duke8253 (via GitHub)" <gi...@apache.org>.
duke8253 commented on issue #9428:
URL: https://github.com/apache/trafficserver/issues/9428#issuecomment-1593721294
> There are many such kind of things. I could file more issues on the project but it'd be overwhelming.
Yeah, I'm just confirming I didn't miss anything, I'll work on that next. And I agree having a _push_stream_dispatcher feels better. I'll work on that as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on issue #9428: Prohibit a second SETTINGS frame
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on issue #9428:
URL: https://github.com/apache/trafficserver/issues/9428#issuecomment-1593710273
> I noticed that we don't seem to enforce one control stream per connection?
There are many such kind of things. I could file more issues on the project but it'd be overwhelming.
> Also maybe we should rename _control_stream_dispatcher to something else, since we use it to handle PUSH streams too?
That is an option. We could also introduce _push_stream_dispatcher, but I'm not too sure if it's a good way of handling those. As you can see, current design is extensible for future extension frames but at the same time it's not great if many frame handlers have to be registered to one dispatcher. I'd say it's nice to have three dispatchers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] duke8253 closed issue #9428: Prohibit a second SETTINGS frame
Posted by "duke8253 (via GitHub)" <gi...@apache.org>.
duke8253 closed issue #9428: Prohibit a second SETTINGS frame
URL: https://github.com/apache/trafficserver/issues/9428
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org