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

HTTP2 Push Stream always results in aborted event being fired #22851

Closed
jgautier opened this Issue Sep 13, 2018 · 0 comments

Comments

Projects
None yet
2 participants
@jgautier

jgautier commented Sep 13, 2018

  • Version: v10.9.0
  • Platform: Darwin Kernel Version 17.4.0: Sun Dec 17 09:19:54 PST 2017; root:xnu-4570.41.2~1/RELEASE_X86_64 x86_64
  • Subsystem: HTTP2

Code sample:

const http2 = require('http2')
const server = http2.createServer()
server.on('stream', (stream, headers) => {
  stream.pushStream({ ':path': '/test' }, (err, pushStream) => {
    if (err) throw err
    pushStream.end('pushed some data')
  })
  stream.end('some data')
})
server.listen(8080, () => {
  let client = http2.connect('http://localhost:8080')
  let req = client.request({ ':path': '/' })
  req.on('data', (data) => {
    console.log(data.toString('utf8'))
  })
  client.on('stream', (pushedStream) => {
    console.log('on stream')
    pushedStream.on('aborted', () => {
      console.log('aborted')
    })
    pushedStream.on('end', () => {
      console.log('end')
    })
    pushedStream.on('data', (data) => {
      console.log(data.toString('utf8'))
    })
  })
})

Output:

on stream
some data
pushed some data
end
aborted

Expectation:

aborted event is not fired

@apapirovski apapirovski added the http2 label Sep 14, 2018

@addaleax addaleax closed this in e72c6af Sep 22, 2018

targos added a commit that referenced this issue Sep 23, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

PR-URL: #22878
Fixes: #22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

kjin added a commit to kjin/node that referenced this issue Sep 27, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

PR-URL: nodejs#22878
Fixes: nodejs#22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

kjin added a commit to kjin/node that referenced this issue Oct 3, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

PR-URL: nodejs#22878
Fixes: nodejs#22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

kjin added a commit to kjin/node that referenced this issue Oct 3, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

PR-URL: nodejs#22878
Fixes: nodejs#22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

kjin added a commit to kjin/node that referenced this issue Oct 3, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

PR-URL: nodejs#22878
Fixes: nodejs#22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

kjin added a commit to kjin/node that referenced this issue Oct 4, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

PR-URL: nodejs#22878
Fixes: nodejs#22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

kjin added a commit to kjin/node that referenced this issue Oct 16, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

PR-URL: nodejs#22878
Fixes: nodejs#22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

BethGriggs added a commit to BethGriggs/node that referenced this issue Oct 16, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

Backport-PR-URL: nodejs#22850
PR-URL: nodejs#22878
Fixes: nodejs#22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

BethGriggs added a commit that referenced this issue Oct 17, 2018

http2: do not falsely emit 'aborted' on push
A push stream should have its writable side closed upon receipt,
to avoid emitting the 'aborted' event when the readable side
is closed.

Backport-PR-URL: #22850
PR-URL: #22878
Fixes: #22851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment