-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add support for auth with custom schemes #62
base: master
Are you sure you want to change the base?
Conversation
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.
Good work, have few comments
src/mgclient.c
Outdated
return extra; | ||
} | ||
|
||
mg_value *scheme_ = mg_value_make_string(scheme ? scheme : "basic"); |
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.
When would scheme
return false?
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.
I guess when mg_value_make_string
fails (e.g., allocation, but unlikely + when it fails since it's small, there are bigger problems most likely)?
|
||
if (username) { |
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.
So the change is also that username and password won't be inserted to extra
if empty?
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.
What do you mean here?
uint32_t version = htobe32(0x0104); | ||
ASSERT_EQ(SendData(sockfd, (char *)&version, 4), 0); | ||
} | ||
uint32_t version = htobe32(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.
why changed from 0x0104 to 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.
Fixed
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.
if 0x0104
is here the tests never finishes
&mg_system_allocator); | ||
mg_session *session = mg_session_init(&mg_system_allocator); | ||
ASSERT_TRUE(session); | ||
session->version = 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.
version from 4 to 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.
Fixed
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.
Yea these seems like hacks to make the client work 🤔
|
||
// Read HELLO message. | ||
// Read INIT message. |
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.
Test is changed from sending HELLO msg to INIT msg, any specific reason?
This PR extends the processing of Bolt v4 HELLO message to support custom auth schemes. The purpose is twofold:
scheme
field)TODOs