Fixed the bug in redirect exposed in #1049

[ShraddheyaS]

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows Code of conduct

[mcollina]

Thanks! Would you mind adding some unit tests as well?

[mcollina]

Can you please use a _hasStatusCode = true property instead?

[mcollina]

I would avoid this block, and avoid calling code(code).

[delvedor]

You can directly use this._hasStatusCode, without changing the headers object :)

[delvedor]

You must initialize it here:

fastify/lib/reply.js

Lines 24 to 33 in 3696096

	function Reply (res, context, request) {
	this.res = res
	this.context = context
	this.sent = false
	this._serializer = null
	this._customError = false
	this._isError = false
	this.request = request
	this._headers = {}
	}

The default value should be false.

[delvedor]

You should also add

this._hasStatusCode = false

to the Reply constructor.

[ShraddheyaS]

While writing the unit tests, does the case like res.code(302).redirect(200, '/') need to be handled?

[mcollina]

Yes I think so

[delvedor]

I think you can refactor the function in this way:

Reply.prototype.redirect = function (code, url) {
  if (typeof code === 'string') {
    url = code
    code = this._hasStatusCode ? this.res.statusCode : 302
  }

  this.header('location', url).code(code).send()
}

Does it work?

[mcollina]

LGTM

[delvedor]

LGTM

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.