feat: follow redirects in SSE client #104
Conversation
Follows redirects in limited situations. Cannot go from http to https or other way around.
|
This pull request has been linked to Shortcut Story #195297: Implement support for HTTP redirects in eventsource. |
cwaldren-ld
changed the title
| } | ||
|
|
||
| return base; | ||
| } |
There was a problem hiding this comment.
I see the AppendUrl function in common, but I'm not sure I need it here since these redirects are probably not human input.
I think it suffices to do:
- If it has a scheme, it's absolute, so that's the new location
- Otherwise, it's relative so try and resolve it against the base URL.
There was a problem hiding this comment.
So, it probably will not be a problem, but a relative URL is relative to the target URI, not the base URL. It is not uncommon for the redirect to have "../" and require dot resolution.
https://www.rfc-editor.org/rfc/rfc3986#section-5.2
"agent MUST process the redirection as if the value inherits the fragment component of the URI reference used to generate the target URI "
There was a problem hiding this comment.
Not as easy to real-world test as by far the most common redirect is http to https.
There was a problem hiding this comment.
(If you have a bunch of instances, and you change an endpoint name, and the endpoints are already segregated by a fragment, then you just use a relative fragment. Not just user input case. I would be comfortable for client SDKs I think. But for server SDKs I would want to solve this.)
This PR allows the SSE client to follow temporary/permanent redirects.
Going from http to https is not going to work because the initial client wouldn't have the ssl context, but this should cover the basic case.
I've also run this revision 164 times through contract tests and didn't observe a crash yet.