Make header.Set reset cookies instead of adding them #1864
Conversation
There was a problem hiding this comment.
The setSpecialHeader method is used in both the Set and Add methods of Header, so modifying it will affect its usage in Add. To reduce code duplication, an additional parameter can be added to setSpecialHeader, or h.Cookies can be reset before calling setSpecialHeader based on the parent method. Resetting h.Cookies can be done using RequestHeader.DelAllCookies's zero slice strategy, as using make may result in unnecessary memory allocation.
Such a pull request may not necessarily be accepted by @erikdubbelboer, but personally, I would accept it. Why not give it a try? 😊
There was a problem hiding this comment.
Thank you very much! I didn't notice that Add also uses the setSpecialHeader method, sorry for that oversight. I reimplemented the process of resetting cookies based on your suggestion.
I also noticed that ResponseHeader should also have corresponding changes, so I changed it accordingly. In addition, I added tests for each case.
There was a problem hiding this comment.
I reviewed the downstream issue you mentioned.
gofiber/fiber#3089 should be fixed under your two pull requests: this and #1860.
gofiber/fiber#3089 (comment)
Error: "[cookieOne=valueCookieOne cookieTwo=valueCookieTwo 0ookieOne=valueCookieOne cookieTwo=valueCookieTwo]" should have 2 item(s), but has 4
You're great!
Hi Developers,
As described in this issue, currently
header.Setdoes not reset but adds cookies. This pull request attempts to fix this so thatheader.Set resetscookies instead of adding them.Feel free to let me know if there are any further changes you would like me to make.
Best regards!