-
Notifications
You must be signed in to change notification settings - Fork 21
Pre-cpprest-removal: Extract http parser into common code #61
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
Conversation
Consume it in SDK.
| std::string firstLine{_incomingDataBuf.begin(), itCR}; | ||
| // std::cout << "Request/Status line: " << firstLine << std::endl; | ||
|
|
||
| static const std::regex rxRequestLine{"([a-zA-Z]+) ([a-zA-Z0-9\\-_\\.!~\\*'\\(\\)%:@&=\\+$,/?]+) [hHtTpP/1\\.]+"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| unsigned int StatusCode() const { return _parsedData->statusCode; } | ||
| std::stringstream& Body() { return _parsedData->body; } | ||
| const std::shared_ptr<HttpPacket>& ParsedData() const { return _parsedData; } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding some unit tests for the parser #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, will do it after code complete. For now, the sdk/agent interaction in all the downloads is sufficient.
| while (_ParseNextField()) | ||
| { | ||
| } | ||
| break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty because we're not interested in anything here for the rest interface? #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fields here refers to the http headers correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah nvm, see the comment in line 128
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, the http headers. We aren't interested in anything other than Content-Length.
jimson-msft
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
No description provided.