Add headerRedirect option #1

ryanburnette · 2018-08-21T14:01:11Z

ryanburnette прокомментировал(а)

2018-08-21 14:01:11 +00:00

Участник

Описание отсутствует.

ryanburnette изменил(а) заголовок с ~~Test~~ на Add headerRedirect option

2018-08-21 14:01:26 +00:00

ryanburnette прокомментировал(а)

2018-08-21 14:11:18 +00:00

Автор

Участник

I propose that there be a headerRedirect option in case someone wants to use header redirects.

I respect your assertion that meta redirect is the best choice. I just think we should have the option in greenlock-express.

It would work something like this:

app.use('/', require('redirect-https')({
  headerRedirect: true
}));

... or ...

app.use('/', require('redirect-https')({
  headerRedirect: {
    responseCode: 307
  }
}));

I propose that there be a `headerRedirect` option in case someone wants to use header redirects. I respect your assertion that meta redirect is the best choice. I just think we should have the option in greenlock-express. It would work something like this: ```js app.use('/', require('redirect-https')({ headerRedirect: true })); ``` ... or ... ```js app.use('/', require('redirect-https')({ headerRedirect: { responseCode: 307 } })); ```

ryanburnette прокомментировал(а)

2018-08-21 14:12:48 +00:00

Автор

Участник

The readme could be updated to say more broadly that this repository is for HTTPS redirect, not just HTTPS redirect using meta.

The documentation should still explain why meta is better in the opinion of the author, citing the blog post, but will offer header as an option and provide a usage example to that end.

The readme could be updated to say more broadly that this repository is for HTTPS redirect, not just HTTPS redirect using meta. The documentation should still explain why meta is better in the opinion of the author, citing the blog post, but will offer header as an option and provide a usage example to that end.

coolaj86 прокомментировал(а)

2018-08-23 04:17:06 +00:00

Владелец

A nit! A nit! I have a nit!

res.redirect only works in express. It won't work in other frameworks like koa, hapi, rill, etc.

Also, for reasons that I don't remember you do need to have a certain minimum number of bytes in the body of a redirect (which express probably does automatically).

I think this is what we need:

res.statusCode = code || 302;
res.end("some escaped html, probably the same as returned regularly");
return;

Would you mind updating and testing?

A nit! A nit! I have a nit! `res.redirect` only works in express. It won't work in other frameworks like koa, hapi, rill, etc. Also, for reasons that I don't remember you do need to have a certain minimum number of bytes in the body of a redirect (which express probably does automatically). I think this is what we need: ```js res.statusCode = code || 302; res.end("some escaped html, probably the same as returned regularly"); return; ``` Would you mind updating and testing?

ryanburnette прокомментировал(а)

2018-08-25 06:01:02 +00:00

Автор

Участник

I don't mind. I will.

ryanburnette прокомментировал(а)

2018-09-07 19:31:46 +00:00

Автор

Участник

I think this is a working PR now. I tried and failed to write a test. You may have to write it for me.

coolaj86 закрыл этот запрос на слияние

2018-10-02 22:59:42 +00:00

Войдите, чтобы присоединиться к обсуждению.

2 участников

Уведомления

Срок выполнения

Срок выполнения не установлен.

Зависимости

Зависимостей нет.

Ссылка: coolaj86/redirect-https.js#1