-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Workers] Update "Custom Ports" known issue to reflect dev-time only behaviour #9324
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
The change made in PR cloudflare#7514 assumes that custom ports work with HTTP requests, however, this is not true. PR cloudflare#5255 did correctly mention the dev-time discrepancy but it was not merged.
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.
Thanks for flagging this. Small copy edits but 🚀
@mrbbot @admah @lrapoport-cf for visibility — wonder if we should update local dev / Quick Editor to more closely mirror production behavior.
|
@irvinebroque tempted to leave it as-is in local dev. We log a warning when you try to |
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.
PCX
|
+1 to the comment from @mrbbot. As long as it's documented here, I'm good with leaving the local dev experience as-is. |
…behaviour (#9324) * Update "Custom Ports" known issue to reflect dev-time only behaviour The change made in PR #7514 assumes that custom ports work with HTTP requests, however, this is not true. PR #5255 did correctly mention the dev-time discrepancy but it was not merged. * Apply suggestions from code review --------- Co-authored-by: Brendan Irvine-Broque <brendanib@gmail.com> Co-authored-by: Brendan Irvine-Broque <birvine-broque@cloudflare.com>
The change made in PR #7514 assumes that custom ports work with HTTP requests, however, this is not true.
PR #5255 did correctly mention the dev-time discrepancy but it was not merged.
Weirdly, it is possible to open up a TCP socket to the HTTP server on the custom port using the recent TCP sockets runtime API, and craft a HTTP request + parse the HTTP response by hand.
Because of this, the decision to restrict custom ports when using
fetchmay be now be defeated by this new TCP socket API?