Skip to content

Conversation

Sightem
Copy link
Contributor

@Sightem Sightem commented Apr 17, 2025

This PR refactors CookieParser::before_handle to use std::string_view for parsing the Cookie request header, avoiding extra std::string allocations during slicing and trimming

Tested against the original code with around 20 inputs, confirming identical output maps and rejection behavior.
Closes #1023

@Sightem
Copy link
Contributor Author

Sightem commented Apr 17, 2025

I tested the implementation against the old one using these:

std::vector<std::string> test_cookies = {
   // Basic cases
   "key=value",
   "key1=value1; key2=value2",
   " key=value ",
   "key = value ; nextkey = anothervalue ",
   // Quoted values
   "key=\"value\"",
   "key=\"\"",
   " key = \" spaced value \" ",
   "key=\"value with ; inside\"",
   "key=\"value with = inside\"",
   "key=",
   "key=;next=val",
   "key= ; next=val",
   // Edge cases
   "",
   "key",
   "=", 
   "=value",
   " key = ",
   "key=value;"
   ";key=value",
   "key1=value1;;key2=value2",
   "key= value ",
   "key=\" value \"",
   "utf8key=你好世界; other=val",
   "special=!@#$%^&*()_+",
   "SID=abc; something=\"\"; key3=val3"
}; 

@Sightem Sightem force-pushed the refactor/cookie-parser-string-view branch 2 times, most recently from b7b229e to 7fb3152 Compare April 20, 2025 21:03
@gittiver
Copy link
Member

OSX uses a newer version of asio which has some deprecated functions removed.

@gittiver
Copy link
Member

gittiver commented May 8, 2025

can you rebase the pull request to newest main, there was a problem with deprecated functions in asio, this let all osx builds fail due to a newer version of asio used there.

@gittiver gittiver added the need_rebase PR needs rebase on master branch label May 20, 2025
@gittiver
Copy link
Member

Could you rebase on current master?

@Sightem Sightem force-pushed the refactor/cookie-parser-string-view branch from 7fb3152 to 588bb65 Compare July 23, 2025 21:24
@Sightem
Copy link
Contributor Author

Sightem commented Jul 23, 2025

My apologies, github decided to never notify me of these comments

@gittiver
Copy link
Member

no worries, i will also need some time to proceed on it. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need_rebase PR needs rebase on master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use std::string_view in CookieParser::before_handle
2 participants