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.

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

Jump to bottom

Initial support proposal for http2 #3390

Closed
wants to merge 12 commits into from
Closed

Initial support proposal for http2 #3390

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
58b88f1
Initial support proposal for http2
PiniH Aug 9, 2017
43f7c01
Seperate to files and revert breaking api
phouri Sep 1, 2017
f98568f
Require parseurl and don't emit warning on setter
phouri Sep 1, 2017
17efcbf
Lint
phouri Sep 1, 2017
b9276c7
Http2 Example
phouri Sep 1, 2017
2e1e78b
Lint...
phouri Sep 1, 2017
878bf5f
Fix example
phouri Sep 3, 2017
e0552bd
Remove try/catch in example
phouri Sep 3, 2017
d3620d2
Remove defineGetter from utils
phouri Sep 3, 2017
e424883
Rename function name
phouri Sep 3, 2017
be2aed1
Remove overridding setter for statusMessage - will land in Node soon
phouri Sep 5, 2017
2ac75f2
Remove unneeded decorators after node js fixes
phouri Sep 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var debug = require('debug')('express:application');
var View = require('./view');
var http = require('http');
var compileETag = require('./utils').compileETag;
var isHttp2Suported = require('./utils').isHttp2Supported;
var compileQueryParser = require('./utils').compileQueryParser;
var compileTrust = require('./utils').compileTrust;
var deprecate = require('depd')('express');
Expand Down Expand Up @@ -99,6 +100,13 @@ app.defaultConfiguration = function defaultConfiguration() {
setPrototypeOf(this.response, parent.response)
setPrototypeOf(this.engines, parent.engines)
setPrototypeOf(this.settings, parent.settings)

//set prototype for http2 requests/response
if (isHttp2Suported) {
setPrototypeOf(this.http2Request, parent.http2Request)
setPrototypeOf(this.http2Response, parent.http2Response)
}

});

// setup locals
Expand Down
21 changes: 17 additions & 4 deletions lib/express.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var Route = require('./router/route');
var Router = require('./router');
var req = require('./request');
var res = require('./response');

var isHttp2Supported = require('./utils').isHttp2Supported;
/**
* Expose `createApplication()`.
*/
Expand All @@ -34,23 +34,36 @@ exports = module.exports = createApplication;
*/

function createApplication() {
var app = function(req, res, next) {
var app = function (req, res, next) {
app.handle(req, res, next);
};

mixin(app, EventEmitter.prototype, false);
mixin(app, proto, false);

// expose the prototype that will get set on requests
app.request = Object.create(req, {
app.request = Object.create(req.req, {
app: { configurable: true, enumerable: true, writable: true, value: app }
})

// expose the prototype that will get set on responses
app.response = Object.create(res, {
app.response = Object.create(res.res, {
app: { configurable: true, enumerable: true, writable: true, value: app }
})

if (isHttp2Supported) {
app.http2Request = Object.create(req.http2Req, {
app: { configurable: true, enumerable: true, writable: true, value: app }
});

app.http2Response = Object.create(res.http2Res, {
app: { configurable: true, enumerable: true, writable: true, value: app }
});
}




app.init();
return app;
}
Expand Down
17 changes: 13 additions & 4 deletions lib/middleware/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,18 @@
*/

'use strict';

/**
* Module dependencies.
* @private
*/

var setPrototypeOf = require('setprototypeof')
var setPrototypeOf = require('setprototypeof');
var isHttp2Supported = require('../utils').isHttp2Supported;
var http2Request = null;

if (isHttp2Supported) {
http2Request = require('http2').Http2ServerRequest;
}
/**
* Initialization middleware, exposing the
* request and response to each other, as well
Expand All @@ -31,9 +35,14 @@ exports.init = function(app){
req.res = res;
res.req = req;
req.next = next;
if (isHttp2Supported && req instanceof http2Request) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be checked by looking at req.httpVersionMajor instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this:
What if spdy-http2/node-spdy#316 is indeed fixed yet node-spdy still uses the http1 IncomingMessage?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually seems fine now that I have fresh eyes. I don't know why I thought it was an issue anyway, I guess just stared at the code too long.

setPrototypeOf(req, app.http2Request)
setPrototypeOf(res, app.http2Response)
} else {
setPrototypeOf(req, app.request)
setPrototypeOf(res, app.response)
}

setPrototypeOf(req, app.request)
setPrototypeOf(res, app.response)

res.locals = res.locals || Object.create(null);

Expand Down