removeRefererHeader docs

[calibr]

in documentation:

removeRefererHeader - removes the referer header when a redirect happens (default: false).

but actually it doesn't remove Referer, it just reuses its value from initial request(like browsers behave). I guess that need to do some corrections in docs about this option.

[simov]

Can you provide a short example or a failing test? Here is the current test regarding that behavior.

[calibr]

ok, I'll do it soon

[simov]

Also note that earlier today I submitted another PR attemting to fix the referer header behavior #1857

[calibr]

if add referer header in test you selected it fails:

tape('should not have a referer when removeRefererHeader is true', function(t) {
  request.post({
    uri: s.url + '/temp',
    jar: jar,
    followAllRedirects: true,
    removeRefererHeader: true,
    headers: { cookie: 'foo=bar', referer: 'http://example.com' }
  }, function(err, res, body) {
    t.equal(err, null)
    t.equal(res.statusCode, 200)
    t.end()
  })
  .on('redirect', function() {
    t.equal(this.headers.referer, undefined)
  })
})

# should not have a referer when removeRefererHeader is true
not ok 77 should be equal
    operator: equal
    expected: |-
      undefined
    actual: |-
      'http://example.com'

so it hasn't removed the referer header as documentation says

[simov]

Right, now I see. As I understand it, the intention was probably to actually remove it. So instead of having:

if (!self.removeRefererHeader) {
  request.setHeader('referer', uriPrev.href)
}

It should be:

if (self.removeRefererHeader) {
  request.removeHeader('referer')
} else {
  request.setHeader('referer', uriPrev.href)
}

Of course the test should be fixed too. What do you think?

[calibr]

I think that current version of removeRefererHeader=true provides the way to process Referer as browsers do it, just preserve Referer from initial request, and in my opinion it's a reasonable behaviour. so I don't think that this behaviour should be changed, because some library users may occur broken if they use this option to behave in redirects like a browser. Probably it should be only a correction in the docs to describe this option more clearly.

[simov]

Yep, I think the current implementation is good. We just need additional test for the case when you manually set the referer header in the initial request. I think I'm going to add one tomorrow.

As for the docs, I'm open for suggestions:

removeRefererHeader - removes the referer header when a redirect happens (default: false). Note: referer header set in the initial request is preserved during redirect.

[calibr]

👍

little suggestion:

removeRefererHeader - removes the referer header when a redirect happens (default: false). Note: if true, referer header set in the initial request is preserved during redirect chain.

(added "if true" and "chain")

[simov]

Fixed in #1860