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

http2: do not allow socket operations on session.socket #16252

Closed
mcollina opened this issue Oct 17, 2017 · 2 comments
Closed

http2: do not allow socket operations on session.socket #16252

mcollina opened this issue Oct 17, 2017 · 2 comments
Assignees
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@mcollina
Copy link
Member

In http2, the session exposes a socket property. However this socket is completely managed by HTTP2 and it is exposed only for information retrieval purposes. The socket itself should not be tampered with, as it can result in unpredictable state.

In the compatibility layer, we are wrapping it in a Proxy, and we are disabling some of the problematic methods (pause(), resume(), etc). I think we should do it in the core API as well.

  • Version: master
  • Platform: all
  • Subsystem: http2

Ref: #16211

@mcollina
Copy link
Member Author

cc @jasnell @apapirovski @trivikr

@apapirovski
Copy link
Member

apapirovski commented Oct 17, 2017

I'm planning to work on this later in the week but if someone wants to take it on before I get there, feel free to do so. Just please post a comment to claim it so we don't overlap.

@mscdex mscdex added the http2 Issues or PRs related to the http2 subsystem. label Oct 17, 2017
@apapirovski apapirovski self-assigned this Oct 19, 2017
apapirovski added a commit to apapirovski/node that referenced this issue Oct 24, 2017
Because of the specific serialization and processing
requirements of HTTP/2, sockets should not be
directly manipulated. This forbids any interactions
with destroy, emit, end, once, on, pause, read,
resume and write methods of the socket. It also
redirects setTimeout to session instead of socket.

Fixes: nodejs#16252
Refs: nodejs#16211
addaleax pushed a commit to ayojs/ayo that referenced this issue Oct 26, 2017
Because of the specific serialization and processing requirements
of HTTP/2, sockets should not be directly manipulated. This
forbids any interactions with destroy, emit, end, pause, read,
resume and write methods of the socket. It also redirects
setTimeout to session instead of socket.

PR-URL: nodejs/node#16330
Fixes: nodejs/node#16252
Refs: nodejs/node#16211
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this issue Oct 30, 2017
Because of the specific serialization and processing requirements
of HTTP/2, sockets should not be directly manipulated. This
forbids any interactions with destroy, emit, end, pause, read,
resume and write methods of the socket. It also redirects
setTimeout to session instead of socket.

PR-URL: #16330
Fixes: #16252
Refs: #16211
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this issue Oct 30, 2017
Because of the specific serialization and processing requirements
of HTTP/2, sockets should not be directly manipulated. This
forbids any interactions with destroy, emit, end, pause, read,
resume and write methods of the socket. It also redirects
setTimeout to session instead of socket.

PR-URL: #16330
Fixes: #16252
Refs: #16211
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this issue Oct 31, 2017
Because of the specific serialization and processing requirements
of HTTP/2, sockets should not be directly manipulated. This
forbids any interactions with destroy, emit, end, pause, read,
resume and write methods of the socket. It also redirects
setTimeout to session instead of socket.

PR-URL: #16330
Fixes: #16252
Refs: #16211
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this issue Dec 7, 2017
Because of the specific serialization and processing requirements
of HTTP/2, sockets should not be directly manipulated. This
forbids any interactions with destroy, emit, end, pause, read,
resume and write methods of the socket. It also redirects
setTimeout to session instead of socket.

PR-URL: nodejs/node#16330
Fixes: nodejs/node#16252
Refs: nodejs/node#16211
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants