Use resume gateway url for session resuming#82
Conversation
|
Attempting to resume always results in "Invalid session, can't resume." |
Discord will be using a different URL for resuming purposes, this commit implements that
lcsmuller
force-pushed
the
feat/resume_gateway_url
branch
from
84d24ea to
7c10c0e
Compare
@Anotra Should be fixed now, can you give it a try? |
|
Took a look at it, I think is pretty good. Wanted to verify that the The thing I want to make note of is that on line 705, we index something and subtract 1 from the URL length. If the URL length is |
|
Also, In my own code, I often do out of bounds error checks with macros for every attempt to index any array (among other things). I see you have an |
HackerSmacker
left a comment
HackerSmacker
left a comment
There was a problem hiding this comment.
Alright, I'm good here! Looks good to me.
| @@ -688,8 +701,8 @@ _discord_gateway_session_from_json(struct discord_gateway_session *session, | |||
| int url_len = (int)f->v.len; | |||
|
|
|||
| url_len = snprintf(session->base_url, sizeof(session->base_url), | |||
There was a problem hiding this comment.
Okay, this part here looks good, since it's gonna get OOB-checked after the fact.
| sizeof(gw->session->resume_url), | ||
| "%.*s%s" DISCORD_GATEWAY_URL_SUFFIX, url_len, | ||
| url, ('/' == url[url_len - 1]) ? "" : "/"); | ||
| ASSERT_NOT_OOB(url_len, sizeof(gw->session->resume_url)); |
There was a problem hiding this comment.
Same here as below -- good idea to have some checking in there.
3 participants
Discord will be using a different URL for resuming purposes, this commit
implements that
What?
Use Discord
resume gateway urlfor session resumingWhy?
In the future it is expected this URL to be used for resuming session purposes
How?
The URL is parsed and kept internally, and the client should attempt to reconnect to that URL if it wants to resume to the previous session.
Testing?
The client has been shutdown without sending a WebSockets
CLOSEopcode to Discord, and then managed to resume to the ongoing session.