Closed nabetti1720 closed 4 months ago
Hi @richarddavison . Thank you for your quick reviews.
This review is more challenging than usual, and I am not sure if I have captured the intent. (The translation by AI also didn't capture all the details...)
I am sorry if my reply is off the mark...
Hi @richarddavison . Please check out the major changes we have made to the way we manage Headers. It has made it much simpler.
I think we handled most of it. :)
Hi @richarddavison , Thanks for pointing this out. I applied it immediately.
We need a separator here to connect multiple set-cookies
, but I believe it is a ", "
separator, not a "; "
.
@nabetti1720 please rebase and I'll run workflow and we can merge as soon as it passes, thanks for this fix 🙂
@nabetti1720 please rebase and I'll run workflow and we can merge as soon as it passes, thanks for this fix 🙂
@richarddavison , Rebasing has been completed.
Issue # (if available)
Fixes #374
Description of changes
set-cookie
inHeaders
classHeaders.getSetCookie()
ToDo
into_js
own implementation and retrieves comma-separated values at a time.~ The reason was that theObject::map_to_entries()
thatjs_entries()
was calling was not able to supportHeaderValue
. We did not want to controlHeaderValue
outside of theHeaders
class, so we implemented our ownjs_entries()
.Sample 1
reproduction.js
```javascript // reproduction.js const headers = new Headers(); headers.append("Set-Cookie", "AAA=123; expires=Sun, 10-Nov-2024 12:29:35 GMT; path=/; domain=.sample.com; HttpOnly"); headers.append("Set-Cookie", "BBB=456; expires=Sun, 10-Nov-2024 12:29:35 GMT; path=/; domain=.sample.com; HttpOnly"); headers.append("accept-encoding", "zstd"); headers.append("accept-encoding", "br"); console.log(headers.getSetCookie().length); console.log("---"); console.log(headers.getSetCookie()); console.log("---"); console.log(headers.get("set-cookie")); console.log("---"); console.log(headers.get("accept-encoding")); ```Result of llrt:
Sample 2
fetch.js
```javascript // fetch.js const main = async () => { try { const response = await fetch('https://google.com'); console.log(response.headers.getSetCookie().length); console.log('---'); console.log(response.headers.getSetCookie()); } catch(e) { console.log(e); } } main(); ```result of llrt:
Sample 3
iterate.js
```javascript // iterate.js const headers = { "content-type": "application/json", authorization: "Bearer 1234", }; const h = new Headers(headers); h.append("set-cookie", "AAA=123; expires=Sun, 10-Nov-2024 12:29:35 GMT"); h.append("set-cookie", "BBB=456; expires=Sun, 10-Nov-2024 12:29:35 GMT"); console.log('1. entries/next ---'); const iterator = h.entries(); let next = iterator.next(); console.log(next.value); next = iterator.next(); console.log(next.value); next = iterator.next(); console.log(next.value); next = iterator.next(); console.log(next.value); next = iterator.next(); console.log(next.value); console.log('---'); console.log('2. values ---'); for (const value of h.values()) { console.log(value); } console.log('---'); console.log('3. forEach ---'); h.forEach((value, key) => { console.log(`${key} ==> ${value}`); }); console.log('---'); console.log('4. for/entries ---'); for (const pair of h.entries()) { console.log(`${pair[0]}: ${pair[1]}`); } console.log('---'); ```result of llrt:
Checklist
tests/unit
and/or in Rust for my feature if neededmake fix
to format JS and apply Clippy auto fixesmake check
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.