Sitelet https://github.com/nodejs/node/issues/48962
Skip to content

Extended connect support for Webtransport #48962

Description

@martenrichter

What is the problem this feature will solve?

I am currently considering extending my Webtransport node plugin (https://github.com/fails-components/webtransport) with support for webtransport over http/2.

I have two options, wait for the support in libquiche, which currently provides the http/3 implementation, or use node.js native http/2.

Thankfully to
#22959
node.js already supports the extended connect protocol and I thought I have already everything for an implementation.
However, https://datatracker.ietf.org/doc/draft-ietf-webtrans-http2/ also requires in clause 3.1 a SETTINGS_WEBTRANSPORT_MAX_SESSIONS setting on the client and server side.

As far as I understand, I can not do anything on the javascript side and this one needs to be implemented on node.js itself or even nghttp2, if I understand the code correctly.

Can you consider adding this to node? As it is a tiny feature.... (Of course, if required I may also provide a PR the extended connect PR did not look to hard, but I do not have much overview over node.js internals).

Or do I miss sth and I can in fact do it in js.

What is the feature you are proposing to solve the problem?

Add SETTINGS_WEBTRANSPORT_MAX_SESSIONS in the same way as extended connect is handled.

What alternatives have you considered?

Use libquiche when it is ready also for this?

Activity

  1. added
    http2Issues and PRs related to the http2 subsystem.
    on Jul 29, 2023
  2. bnoordhuis commented on Jul 29, 2023

    @bnoordhuis
    Member

    As a general rule we don't add support for things that are still in draft mode (which webtransport is.)

    Besides that, it's unclear to me what you're actually asking for. SETTINGS_WEBTRANSPORT_MAX_SESSIONS is a h3 thing, not a h2 thing.

    edit: I guess you're asking for something similar to SETTINGS_MAX_CONCURRENT_STREAMS but that's implemented in nghttp2. Node merely exposes it.

  3. martenrichter commented on Jul 29, 2023

    @martenrichter
    ContributorAuthor

    Yes, Webtransport is primarily a http/3 thing. But it has a newer draft specification to map it onto http2.
    This is the draft https://datatracker.ietf.org/doc/draft-ietf-webtrans-http2/ .
    Short summary: it uses the same extended connect mechanism also websocket over http/2 uses, and it is already supported by node.js. The only thing is, that it adds SETTINGS_WEBTRANSPORTMAX_SESSIONS to the settings object.

    So a client would send:

    SETTINGS
    SETTINGS_ENABLE_CONNECT_PROTOCOL = 1
    SETTINGS_WEBTRANSPORT_MAX_SESSIONS = 1
    

    and a server would reply:

    SETTINGS
    SETTINGS_ENABLE_CONNECT_PROTOCOL = 1
    SETTINGS_WEBTRANSPORT_MAX_SESSIONS = 100
    

    This is the only thing from the new spec, I can not implement on top of node (to my current knowledge).
    After this I can just take over the stream and parse the capsule protocol, which is used to map http/3 syntax to http/2.

    Of course, it was clear to me, that you do not want to implement something complex, while it is still in flow. But this one is rather tiny to unlock.
    And yes I know, it is more something for nghttp2, since I have already looked in the code, but I thought I would ask on the top level, since it will not help if it is only implemented in nghttp2.

  4. bnoordhuis commented on Jul 29, 2023

    @bnoordhuis
    Member

    We normally take a bottom-up approach. :-)

    If nghttp2 adds it, then exposing it in node should be uncontroversial.

  5. bnoordhuis commented on Jul 30, 2023

    @bnoordhuis
    Member

    I see the suggestion from upstream is to use nghttp2_submit_settings() and that could work (already used internally) but right now it's wrapped by a Http2Settings object that only understands a hard-coded list of settings.

    Making it understand arbitrary settings requires both a new public API and a possibly not insignificant amount of refactoring. I don't plan to work on it but pull request welcome.

  6. martenrichter commented on Jul 30, 2023

    @martenrichter
    ContributorAuthor

    I was actually about the write more or less the same. But it took me some time to understand the API (I worked more with quiche for the http3 stuff).
    Ok, then I will put this onto my to-do list, may take a while, I am just coding on weekends and I first have to setup up a build environment for node, which is usually the higher hurdle.

    Any suggestions for the arbitrary settings api? My first idea would be an object, the key is the numeric id per spec and value is the value per spec. The other question is, what to do with settings, which are already implemented. So how to sync, for backward compatibility they should be kept.

  7. bnoordhuis commented on Jul 30, 2023

    @bnoordhuis
    Member

    My off-the-cuff thinking is that a prototype method is probably the best way to go about it, because that lets you handle existing properties more easily. Like this basically:

    diff --git a/src/node_http2.cc b/src/node_http2.cc
    index 01d0eb3418b..54c20b2ece6 100644
    --- a/src/node_http2.cc
    +++ b/src/node_http2.cc
    @@ -3231,6 +3231,7 @@ void Initialize(Local<Object> target,
    
       Local<FunctionTemplate> setting = FunctionTemplate::New(env->isolate());
       setting->Inherit(AsyncWrap::GetConstructorTemplate(env));
    +  SetProtoMethod(isolate, setting, "add", Http2Settings::Add);
       Local<ObjectTemplate> settingt = setting->InstanceTemplate();
       settingt->SetInternalFieldCount(AsyncWrap::kInternalFieldCount);
       env->set_http2settings_constructor_template(settingt);
  8. martenrichter commented on Jul 30, 2023

    @martenrichter
    ContributorAuthor

    So maybe like the JS map interface get and set, as the settings will also be received and one might want to query the fields that are set?
    And the key can be either the numeric value or the string literal for known (non-draft) properties?
    Or just the numeric value and a set of constants is provided?

  9. martenrichter commented on Jul 30, 2023

    @martenrichter
    ContributorAuthor

    Ok, I have read nghttp2 source code in more detail.
    Currently, I can only send additional settings. So only an "add" prototype is possible, getters currently do not make sense as nghttp2 does not support it.
    So retrieving the settings from the connected client or server is not supported by nghttp2. as every value is hardcoded. For me, this is fine as I can just assume that the setting in question is supported, all I care about is that the browser gets the correct header.

  10. martenrichter commented on Aug 6, 2023

    @martenrichter
    ContributorAuthor

    Draft PR as the first sketch is now available.

  11. martenrichter commented on Aug 21, 2023

    @martenrichter
    ContributorAuthor

    Ok, the PR now passed the tests and should be ready for review.

  12. github-actions commented on Feb 18, 2024

    @github-actions
    Contributor

    There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

    For more information on how the project manages feature requests, please consult the feature request management document.

  13. added
    staleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.
    on Feb 18, 2024
  14. martenrichter commented on Feb 18, 2024

    @martenrichter
    ContributorAuthor

    Is already fully implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    feature requestIssues requesting new Node.js features.http2Issues and PRs related to the http2 subsystem.staleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions