Skip to content

Fixes errors in the http/2 implementation #383

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.

Sign up for GitHub

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

Open
martenrichter wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixes errors in the http/2 implementation #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
000ee11
Fix bug for zero streamid handling
martenrichter May 17, 2025
e1fefe2
Change http2 stream id convention to match spec
martenrichter May 17, 2025
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
4 changes: 2 additions & 2 deletions main/lib/http2/node/capsuleparser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ export class Http2CapsuleParser extends ParserBaseHttp2 {
if (bufferstate.size < length + offsetbegin) {
this.remainlength = length + offsetbegin - bufferstate.size
this.mode = 'c'
if (streamid) {
if (typeof streamid !== 'undefined') {
this.rstreamid = streamid
this.rfin = type === Http2CapsuleParser.WT_STREAM_WFIN
}
Expand All @@ -270,7 +270,7 @@ export class Http2CapsuleParser extends ParserBaseHttp2 {
bufferstate.size - bufferstate.offset,
this.remainlength
)
if (this.rstreamid) {
if (typeof this.rstreamid !== 'undefined') {
// TODO submitData
const object = this.wtstreams.get(this.rstreamid)
const fin =
Expand Down
4 changes: 2 additions & 2 deletions main/lib/http2/node/websocketparser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ export class WebSocketParser extends ParserBaseHttp2 {
if (bufferstate.size < length + offsetbegin) {
this.remainlength = length + offsetbegin - bufferstate.size
this.mode = 'c'
if (streamid) {
if (typeof streamid !== 'undefined') {
this.rstreamid = streamid
this.rfin = type === ParserBase.WT_STREAM_WFIN
}
Expand All @@ -669,7 +669,7 @@ export class WebSocketParser extends ParserBaseHttp2 {
bufferstate.size - bufferstate.offset,
this.remainlength
)
if (this.rstreamid) {
if (typeof this.rstreamid !== 'undefined') {
// not in j mode
// TODO submitData
const object = this.wtstreams.get(this.rstreamid)
Expand Down
2 changes: 1 addition & 1 deletion main/lib/http2/parserbase.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export class ParserBase {
* @param {{sendOrder: bigint,sendGroupId: bigint}} priority
*/
newStream(streamid, priority) {
const incoming = this.isclient ? !(streamid & 0x1n) : !!(streamid & 0x1n)
const incoming = this.isclient ? !!(streamid & 0x1n) : !(streamid & 0x1n)
const streamIdManager =
streamid & 0x2n
? this.session.streamIdMngrUni
Expand Down
10 changes: 5 additions & 5 deletions main/lib/http2/streamidmanager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ export class StreamIdManager {
onStreamClosed(streamId) {
// Nothing to do for outgoing streams.
if (
(this.isclient && streamId & 0x1n) ||
(!this.isclient && !(streamId & 0x1n))
(this.isclient && !(streamId & 0x1n)) ||
(!this.isclient && streamId & 0x1n)
)
return

Expand Down Expand Up @@ -283,7 +283,7 @@ export class StreamIdManager {
* @param {number} id
*/
isAvailableStream(id) {
if ((this.isclient && id & 0x1) || (!this.isclient && !(id & 0x1))) {
if ((this.isclient && !(id & 0x1)) || (!this.isclient && id & 0x1)) {
// Stream IDs under next_ougoing_stream_id_ are either open or previously
// open but now closed.
return id >= this.nextOutgoingStreamId
Expand All @@ -298,14 +298,14 @@ export class StreamIdManager {

getFirstOutgoingStreamId() {
let streamid = 0n
if (this.isclient) streamid |= 0x1n
if (!this.isclient) streamid |= 0x1n
if (this.unidirectional) streamid |= 0x2n
return streamid
}

getFirstIncomingStreamId() {
let streamid = 0n
if (!this.isclient) streamid |= 0x1n
if (this.isclient) streamid |= 0x1n
if (this.unidirectional) streamid |= 0x2n
return streamid
}
Expand Down
8 changes: 7 additions & 1 deletion main/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@ A web-based lecture system developed out of university lectures.
While FAILS as a whole is licensed via GNU Affero GPL version 3.0, this package is licensed under a BSD-style license that can be found in the LICENSE file, while code taken from other projects is still under their respective license (see LICENSE file for details).
This package is licensed more permissive since it can be useful outside of the FAILS environment.

This module started as a C++ node binding to libquiche [https://github.com/google/quiche](https://github.com/google/quiche) (note there is a second library with a similar purpose and the same name), which provides besides other network protocols HTTP/3 support. Now it can also handle Webtransport over HTTP/2 and a WebSocket mapping of the HTTP/2 protocol together with a polyfill and ponyfill for the browsers without reliable WebTransport support or no WebTRansport support at all.
This module started as a C++ node binding to libquiche [https://github.com/google/quiche](https://github.com/google/quiche) (note there is a second library with a similar purpose and the same name), which provides besides other network protocols HTTP/3 support. Now it can also handle Webtransport over HTTP/2 and a WebSocket mapping of the HTTP/2 protocol together with a polyfill and ponyfill for the browsers without reliable WebTransport support or no WebTransport support at all.
This package currently provides support for WebTransport with an interface similar to the browser side (but not all features implemented), for the server as well as for the client, see `old_test/test.js`, `old_test/testsuite.js`, `old_test/echoclient.js`, `old_test/echoserver.js` for examples.
Note, that the client implementation only supports certificate checking via `certificateHashes`, however experimental support for rootCA-based checking is introduced with version 1.0.0.
It may be possible in the future to also support normal HTTP/3 with not so much effort, however, there is no intention from the author to implement this since it will not be needed by FAILS. However, PR requests are welcome and will be supported by advice from the author.
The package for the HTTP/3 should be considered as a duct tape-style solution until a bulletproof native support of HTTP/3 and WebTransport is provided by node itself.

## Changes for Version 2.x.x

The interface did not change it all.
However input from a Firefox developer uncovered a non standard complaint implementation of the http/2 protocols, that includes the Webtransport over WebSocket polyfill.
Therefore, the protocol of the non http/3 flavors is not compatible to the 1.x.x versions.

## Changes for Version 1.x.x

The interface for version 1.x.x is not changed compared to 0.x.x.
Expand Down