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
refactor: update Reply module #4072
Changes from 2 commits
c544629
d1fe724
4573e1c
6602564
7b515d1
c905725
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -165,6 +165,7 @@ Reply.prototype.send = function (payload) { | |
// If user doesn't set charset, we will set charset to utf-8 | ||
if (contentType.indexOf('charset') === -1) { | ||
const customContentType = contentType.trim() | ||
|
||
if (customContentType.endsWith(';')) { | ||
// custom content-type is ended with ';' | ||
this[kReplyHeaders]['content-type'] = `${customContentType} charset=utf-8` | ||
|
@@ -186,12 +187,8 @@ Reply.prototype.send = function (payload) { | |
|
||
Reply.prototype.getHeader = function (key) { | ||
key = key.toLowerCase() | ||
const res = this.raw | ||
let value = this[kReplyHeaders][key] | ||
if (value === undefined && res.hasHeader(key)) { | ||
value = res.getHeader(key) | ||
} | ||
return value | ||
|
||
return this[kReplyHeaders][key] ?? this.raw.getHeader(key) | ||
jsumners marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
Reply.prototype.getHeaders = function () { | ||
|
@@ -203,10 +200,8 @@ Reply.prototype.getHeaders = function () { | |
|
||
Reply.prototype.hasHeader = function (key) { | ||
key = key.toLowerCase() | ||
if (this[kReplyHeaders][key] !== undefined) { | ||
return true | ||
} | ||
return this.raw.hasHeader(key) | ||
|
||
return this[kReplyHeaders][key] !== undefined || this.raw.hasHeader(key) | ||
} | ||
|
||
Reply.prototype.removeHeader = function (key) { | ||
|
@@ -216,35 +211,32 @@ Reply.prototype.removeHeader = function (key) { | |
return this | ||
} | ||
|
||
Reply.prototype.header = function (key, value) { | ||
const _key = key.toLowerCase() | ||
|
||
// default the value to '' | ||
value = value === undefined ? '' : value | ||
Reply.prototype.header = function (key, value = '') { | ||
key = key.toLowerCase() | ||
|
||
if (this[kReplyHeaders][_key] && _key === 'set-cookie') { | ||
if (this[kReplyHeaders][key] && key === 'set-cookie') { | ||
// https://tools.ietf.org/html/rfc7230#section-3.2.2 | ||
if (typeof this[kReplyHeaders][_key] === 'string') { | ||
this[kReplyHeaders][_key] = [this[kReplyHeaders][_key]] | ||
if (typeof this[kReplyHeaders][key] === 'string') { | ||
this[kReplyHeaders][key] = [this[kReplyHeaders][key]] | ||
} | ||
|
||
if (Array.isArray(value)) { | ||
Array.prototype.push.apply(this[kReplyHeaders][_key], value) | ||
this[kReplyHeaders][key].push(...value) | ||
} else { | ||
this[kReplyHeaders][_key].push(value) | ||
this[kReplyHeaders][key].push(value) | ||
} | ||
} else { | ||
this[kReplyHeaders][_key] = value | ||
this[kReplyHeaders][key] = value | ||
} | ||
|
||
return this | ||
} | ||
|
||
Reply.prototype.headers = function (headers) { | ||
const keys = Object.keys(headers) | ||
/* eslint-disable no-var */ | ||
for (var i = 0; i !== keys.length; ++i) { | ||
const key = keys[i] | ||
this.header(key, headers[key]) | ||
jsumners marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (const headerName in headers) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a significant difference between
In short, the |
||
this.header(headerName, headers[headerName]) | ||
} | ||
|
||
return this | ||
} | ||
|
||
|
@@ -267,25 +259,29 @@ const INVALID_TRAILERS = new Set([ | |
|
||
Reply.prototype.trailer = function (key, fn) { | ||
key = key.toLowerCase() | ||
|
||
if (INVALID_TRAILERS.has(key)) { | ||
throw new FST_ERR_BAD_TRAILER_NAME(key) | ||
} | ||
|
||
if (typeof fn !== 'function') { | ||
throw new FST_ERR_BAD_TRAILER_VALUE(key, typeof fn) | ||
} | ||
if (this[kReplyTrailers] === null) this[kReplyTrailers] = {} | ||
|
||
this[kReplyTrailers] = this[kReplyTrailers] ?? {} | ||
this[kReplyTrailers][key] = fn | ||
|
||
return this | ||
} | ||
|
||
Reply.prototype.hasTrailer = function (key) { | ||
if (this[kReplyTrailers] === null) return false | ||
return this[kReplyTrailers][key.toLowerCase()] !== undefined | ||
return this[kReplyTrailers]?.[key.toLowerCase()] !== undefined | ||
} | ||
|
||
Reply.prototype.removeTrailer = function (key) { | ||
if (this[kReplyTrailers] === null) return this | ||
this[kReplyTrailers][key.toLowerCase()] = undefined | ||
|
||
return this | ||
} | ||
|
||
|
@@ -330,8 +326,7 @@ Reply.prototype.redirect = function (code, url) { | |
code = this[kReplyHasStatusCode] ? this.raw.statusCode : 302 | ||
} | ||
|
||
this.header('location', url).code(code).send() | ||
return this | ||
return this.header('location', url).code(code).send() | ||
} | ||
|
||
Reply.prototype.callNotFound = function () { | ||
|
@@ -485,9 +480,7 @@ function onSendEnd (reply, payload) { | |
} | ||
|
||
if (reply[kReplyTrailers] === null) { | ||
if (!reply[kReplyHeaders]['content-length']) { | ||
reply[kReplyHeaders]['content-length'] = '' + Buffer.byteLength(payload) | ||
} else if (req.raw.method !== 'HEAD' && reply[kReplyHeaders]['content-length'] !== Buffer.byteLength(payload)) { | ||
if (!reply[kReplyHeaders]['content-length'] || (req.raw.method !== 'HEAD' && parseInt(reply[kReplyHeaders]['content-length'], 10) !== Buffer.byteLength(payload))) { | ||
Eomm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
reply[kReplyHeaders]['content-length'] = '' + Buffer.byteLength(payload) | ||
denshakhov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
@@ -568,12 +561,15 @@ function sendStream (payload, res, reply) { | |
|
||
function sendTrailer (payload, res, reply) { | ||
if (reply[kReplyTrailers] === null) return | ||
const trailerHeaders = Object.keys(reply[kReplyTrailers]) | ||
|
||
const trailers = {} | ||
for (const trailerName of trailerHeaders) { | ||
|
||
for (const trailerName in reply[kReplyTrailers]) { | ||
Eomm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (typeof reply[kReplyTrailers][trailerName] !== 'function') continue | ||
|
||
trailers[trailerName] = reply[kReplyTrailers][trailerName](reply, payload) | ||
} | ||
|
||
res.addTrailers(trailers) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is happening here? Is this a line ending change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, just a new line to separate var definition and if condition (I added it when I was refactoring this reverted block of code). Delete?