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

Removal of body and some headers on redirects #2559

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dbursztyn
Copy link

@dbursztyn dbursztyn commented Feb 22, 2017

PR Checklist:

  • I have run npm test locally and all tests are passing.
  • I have added/updated tests for any new behavior.
  • If this is a significant change, an issue has already been created where the problem / solution was discussed: #2464 (please note that an related issue has already been created, however no solution had been discussed)

PR Description

As pointed out in issue #2464 some headers, body and _form are being remove on redirects.

Added options followOriginalBody (boolean) and followOriginalBodyForCodes (list) such that if the response status is in followOriginalBodyForCodes ([401, 307] by default to maintain current behavior) or followOriginalBody is true (false by default to maintain current behavior), then host, content-type and content-length headers, body and _form are not removed.

Added the debug information of the headers, body and _form on redirection as the debug info in request.init(options) contains the options, which are not defined in case of redirections.

Also added some tests to check that in case of redirection the above mentioned headers are being removed when the response status is not in followOriginalBodyForCodes and followOriginalBody is false, and not being removed otherwise. The removal or not of the body and _form should also be tested but couldn't find them in the request (indications on how to test them are welcome and appreciated).

Please find below the debug output, including the empty "REQUEST" and the added "REQUEST redirect with headers", "REQUEST redirect with body" and "REQUEST redirect with _form".

REQUEST { uri: 'http://localhost:56556/temp',
  jar:
   RequestJar {
     _jar:
      CookieJar {
        enableLooseMode: true,
        store:
         { idx: { localhost:
            { '/':
               { quux: Cookie="quux=baz; Path=/; hostOnly=true; aAge=3ms; cAge=178ms",
                 ham: Cookie="ham=eggs; Path=/; hostOnly=true; aAge=3ms; cAge=157ms" } } } } } },
  headers: { cookie: 'foo=bar' },
  agentOptions: {},
  agentClass: [Function: FakeAgent],
  callback: [Function],
  method: 'GET' }
REQUEST make request http://localhost:56556/temp
REQUEST onRequestResponse http://localhost:56556/temp 301 { location: 'http://localhost:56556/temp_landing',
  'set-cookie': [ 'ham=eggs' ],
  date: 'Wed, 22 Feb 2017 14:54:58 GMT',
  connection: 'close',
  'transfer-encoding': 'chunked' }
REQUEST redirect http://localhost:56556/temp_landing
REQUEST redirect to http://localhost:56556/temp_landing
REQUEST redirect with headers { cookie: 'foo=bar; quux=baz; ham=eggs',
  referer: 'http://localhost:56556/temp' }
REQUEST redirect with body undefined
REQUEST redirect with _form undefined
REQUEST {}
REQUEST response end http://localhost:56556/temp_landing 301 { location: 'http://localhost:56556/temp_landing',
  'set-cookie': [ 'ham=eggs' ],
  date: 'Wed, 22 Feb 2017 14:54:58 GMT',
  connection: 'close',
  'transfer-encoding': 'chunked' }
REQUEST make request http://localhost:56556/temp_landing
REQUEST onRequestResponse http://localhost:56556/temp_landing 200 { 'x-response': 'GET temp_landing',
  date: 'Wed, 22 Feb 2017 14:54:58 GMT',
  connection: 'close',
  'transfer-encoding': 'chunked' }
REQUEST reading response's body
REQUEST finish init function http://localhost:56556/temp_landing
REQUEST response end http://localhost:56556/temp_landing 200 { 'x-response': 'GET temp_landing',
  date: 'Wed, 22 Feb 2017 14:54:58 GMT',
  connection: 'close',
  'transfer-encoding': 'chunked' }
REQUEST end event http://localhost:56556/temp_landing
REQUEST has body http://localhost:56556/temp_landing 16
REQUEST emitting complete http://localhost:56556/temp_landing

@mirkods
Copy link
Contributor

mirkods commented Mar 23, 2017

any updates about the merge of it?

@tovafi-zz
Copy link

I would like to see it merged, too.
Any chance of it happening soon?

@Kamikadze4GAME
Copy link

+1 It's best

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants