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.

Sign up for GitHub

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

Jump to bottom

The type of set-cookies obtained by heaers is inconsistent with the actual type? #1892

Closed
JQiue opened this issue Jan 28, 2023 · 0 comments · Fixed by #1896
Closed

The type of set-cookies obtained by heaers is inconsistent with the actual type? #1892

JQiue opened this issue Jan 28, 2023 · 0 comments · Fixed by #1896
Labels
bug Something isn't working

Comments

@JQiue
Copy link

JQiue commented Jan 28, 2023

Bug Description

headers["set-cookie"]returned is a string, type declaration is a string [] | undefined?

 const { headers } = await request(index_url);
 console.log(headers);
/* {
  date: 'Sat, 28 Jan 2023 09:43:48 GMT',
  'content-type': 'text/html',
  'content-length': '2920',
  connection: 'keep-alive',
  vary: 'Accept-Encoding',
  'last-modified': 'Sat, 17 Sep 2022 08:52:52 GMT',
  'accept-ranges': 'bytes',
  etag: '"1d8ca72dec58168"',
  'set-cookie': 'SERVERID=f7a91655448e700d4049e6f0ca2b6b52|1674899028|1674899028;Path=/'
} */
console.log(headers["set-cookie"]);
/*
SERVERID=f7a91655448e700d4049e6f0ca2b6b52|1674899028|1674899028;Path=/
*/

Reproducible By

Expected Behavior

Logs & Screenshots

Environment

Additional context
@JQiue JQiue added the bug Something isn't working label Jan 28, 2023
pan93412 added a commit to pan93412/undici that referenced this issue Jan 28, 2023
@pan93412
Implement the requirement of IncomingHttpHeaders["set-cookie"]
0f409c0 
Currently `headers["set-cookie"]` does not split,
which can't match the expected type of IncomingHttpHeader.

This commit attempts to split `headers["set-cookie"]` by `;`
like what 'node:http' does:
https://nodejs.org/api/http.html#class-httpserverresponse

Fixed nodejs#1892
pan93412 added a commit to pan93412/undici that referenced this issue Jan 28, 2023
@pan93412
Implement the requirement of IncomingHttpHeaders["set-cookie"]
4e54ad0 
Currently `headers["set-cookie"]` does not split,
which can't match the expected type of IncomingHttpHeader.

This commit attempts to split `headers["set-cookie"]` by `;`
like what 'node:http' does:
https://nodejs.org/api/http.html#class-httpserverresponse

Fixed nodejs#1892

Signed-off-by: pan93412 <pan93412@gmail.com>
@pan93412 pan93412 mentioned this issue Jan 28, 2023
Closed
7 tasks
pan93412 added a commit to pan93412/undici that referenced this issue Jan 29, 2023
@pan93412
Correct the type of DispatchOptions["headers"]
35b391f 
According to core/util@parseHeaders, the current behavior is:

- By default, the entry type is a `string`.
- The entry type turns to a `string[]` when there are duplicated entries

This behavior is not as same as what `node:http` does currently;
therefore, `IncomingHttpHeaders` can't reflect our parsed data.

This commit attempts to reflect this parsing behavior.

Fixed nodejs#1892

Signed-off-by: pan93412 <pan93412@gmail.com>
@pan93412 pan93412 mentioned this issue Jan 29, 2023
Merged
7 tasks
pan93412 added a commit to pan93412/undici that referenced this issue Jan 29, 2023
@pan93412
Correct the type of DispatchOptions["headers"]
4bc791f 
According to core/util@parseHeaders, the current behavior is:

- By default, the entry type is a `string`.
- The type turns to a `string[]` when there are duplicated entries.

This behavior is not as same as what `node:http` does currently;
therefore, `IncomingHttpHeaders` can't reflect our parsed data.

This commit attempts to reflect this parsing behavior
by redefining it to `Record<string, string | string[]>`.

Fixed nodejs#1892
The simpler solution of nodejs#1893

Signed-off-by: pan93412 <pan93412@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
1 participant
@JQiue