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

Duplicate header handling #874

Merged
merged 4 commits into from Aug 12, 2017
Merged

Conversation

@tybro0103
Copy link
Contributor

@tybro0103 tybro0103 commented Apr 29, 2017

Better handling of duplicate headers:

  • Ignore when name is in this list
  • Use an array for Set-Cookie
  • Otherwise join with ,

Fixes: #465

Just finishing up (added tests) kovensky's work (#558)... thanks @kovensky!

Diogo Franco and others added 3 commits Nov 28, 2016
Node ignores duplicate entries for certain HTTP headers.

It also always converts the `set-cookie` header into an array.
@coveralls
Copy link

@coveralls coveralls commented Apr 29, 2017

Coverage Status

Coverage increased (+1.6%) to 93.797% when pulling 48c4103 on tybro0103:duplicate-header-handling into f31317a on mzabriskie:master.

@tybro0103
Copy link
Contributor Author

@tybro0103 tybro0103 commented Apr 29, 2017

Unfortunately, a breaking change. Hope that doesn't delay getting this merged?

'Set-Cookie: key2=val2;\n'
);

expect(parsedZero['set-cookie']).toBeUndefined();

This comment has been minimized.

@tybro0103

tybro0103 Apr 29, 2017
Author Contributor

Not sure if it'd be better to have any empty array here or not

@coveralls
Copy link

@coveralls coveralls commented Apr 29, 2017

Coverage Status

Coverage increased (+1.6%) to 93.797% when pulling d29962a on tybro0103:duplicate-header-handling into f31317a on mzabriskie:master.

@tybro0103
Copy link
Contributor Author

@tybro0103 tybro0103 commented Aug 10, 2017

@rubennorte Any chance this could get merged?

@rubennorte rubennorte merged commit fb08e95 into axios:master Aug 12, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rubennorte
Copy link
Member

@rubennorte rubennorte commented Aug 12, 2017

@tybro0103 and @Kovensky thanks for your contributions!

Gerhut added a commit to Gerhut/axios that referenced this pull request Aug 13, 2017
* Update parseHeaders to match node http behavior

Node ignores duplicate entries for certain HTTP headers.

It also always converts the `set-cookie` header into an array.

* add tests for new duplicate header handling

* clarify comment
jimthedev added a commit to commitizen/cz-cli that referenced this pull request May 24, 2018
This Pull Request updates dependency [axios](https://github.com/axios/axios) from `v0.15.2` to `v0.18.0`



<details>
<summary>Release Notes</summary>

### [`v0.18.0`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0180-Feb-19-2018)
[Compare Source](axios/axios@v0.17.1...v0.18.0)
- Adding support for UNIX Sockets when running with Node.js ([#&#8203;1070](`axios/axios#1070))
- Fixing typings ([#&#8203;1177](`axios/axios#1177)):
    - AxiosRequestConfig.proxy: allows type false
    - AxiosProxyConfig: added auth field
- Adding function signature in AxiosInstance interface so AxiosInstance can be invoked ([#&#8203;1192](`axios/axios#1192), [#&#8203;1254](`axios/axios#1254))
- Allowing maxContentLength to pass through to redirected calls as maxBodyLength in follow-redirects config ([#&#8203;1287](`axios/axios#1287))
- Fixing configuration when using an instance - method can now be set ([#&#8203;1342](`axios/axios#1342))

---

### [`v0.17.1`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0171-Nov-11-2017)
[Compare Source](axios/axios@v0.17.0...v0.17.1)
- Fixing issue with web workers ([#&#8203;1160](`axios/axios#1160))
- Allowing overriding transport ([#&#8203;1080](`axios/axios#1080))
- Updating TypeScript typings ([#&#8203;1165](`axios/axios#1165), [#&#8203;1125](`axios/axios#1125), [#&#8203;1131](`axios/axios#1131))

---

### [`v0.17.0`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0170-Oct-21-2017)
[Compare Source](axios/axios@v0.16.2...v0.17.0)
- **BREAKING** Fixing issue with `baseURL` and interceptors ([#&#8203;950](`axios/axios#950))
- **BREAKING** Improving handing of duplicate headers ([#&#8203;874](`axios/axios#874))
- Adding support for disabling proxies ([#&#8203;691](`axios/axios#691))
- Updating TypeScript typings with generic type parameters ([#&#8203;1061](`axios/axios#1061))

---

### [`v0.16.2`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0162-Jun-3-2017)
[Compare Source](axios/axios@v0.16.1...v0.16.2)
- Fixing issue with including `buffer` in bundle ([#&#8203;887](`axios/axios#887))
- Including underlying request in errors ([#&#8203;830](`axios/axios#830))
- Convert `method` to lowercase ([#&#8203;930](`axios/axios#930))

---

### [`v0.16.1`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0161-Apr-8-2017)
[Compare Source](axios/axios@v0.16.0...v0.16.1)
- Improving HTTP adapter to return last request in case of redirects ([#&#8203;828](`axios/axios#828))
- Updating `follow-redirects` dependency ([#&#8203;829](`axios/axios#829))
- Adding support for passing `Buffer` in node ([#&#8203;773](`axios/axios#773))

---

### [`v0.16.0`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0160-Mar-31-2017)
[Compare Source](axios/axios@v0.15.3...v0.16.0)
- **BREAKING** Removing `Promise` from axios typings in favor of built-in type declarations ([#&#8203;480](`axios/axios#480))
- Adding `options` shortcut method ([#&#8203;461](`axios/axios#461))
- Fixing issue with using `responseType: 'json'` in browsers incompatible with XHR Level 2 ([#&#8203;654](`axios/axios#654))
- Improving React Native detection ([#&#8203;731](`axios/axios#731))
- Fixing `combineURLs` to support empty `relativeURL` ([#&#8203;581](`axios/axios#581))
- Removing `PROTECTION_PREFIX` support ([#&#8203;561](`axios/axios#561))

---

### [`v0.15.3`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0153-Nov-27-2016)
[Compare Source](axios/axios@v0.15.2...v0.15.3)
- Fixing issue with custom instances and global defaults ([#&#8203;443](`axios/axios#443))
- Renaming `axios.d.ts` to `index.d.ts` ([#&#8203;519](`axios/axios#519))
- Adding `get`, `head`, and `delete` to `defaults.headers` ([#&#8203;509](`axios/axios#509))
- Fixing issue with `btoa` and IE ([#&#8203;507](`axios/axios#507))
- Adding support for proxy authentication ([#&#8203;483](`axios/axios#483))
- Improving HTTP adapter to use `http` protocol by default ([#&#8203;493](`axios/axios#493))
- Fixing proxy issues ([#&#8203;491](`axios/axios#491))

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
@opichals
Copy link

@opichals opichals commented Apr 1, 2019

@rubennorte @tybro0103 Why is the Set-Cookie an exception here?

One would expect any header field value could be set via an array to have duplicate field-name entries. For node.js this works as documented https://nodejs.org/api/http.html#http_request_setheader_name_value)

@axios axios locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants