Skip to content
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

Upstream Scheme is not applied to DownstreamScheme, if latter is not specified #1509

Closed
abhiphirke opened this issue Aug 27, 2021 · 5 comments · Fixed by #1689
Closed

Upstream Scheme is not applied to DownstreamScheme, if latter is not specified #1509

abhiphirke opened this issue Aug 27, 2021 · 5 comments · Fixed by #1689
Assignees
Labels
bug Identified as a potential bug merged Issue has been merged to dev and is waiting for the next release Websockets Ocelot feature: Websockets

Comments

@abhiphirke
Copy link

abhiphirke commented Aug 27, 2021

Expected Behavior / New Feature

If DownstreamScheme is not specified in a Route in configuration, the upstream scheme should be used to forward request to the downstream for the URLs other than http/https.

Basically, I want to support long polling as well as websockets on a SignalR hub/client in my application.
In both cases, I have to hit the same URL, except that my up and down stream schemes change.
I cannot create two entries with same url but different DownstreamScheme s at the same time. [I hope my understanding is right here. If not, please enlighten me. :) ]

Actual Behavior / Motivation for New Feature

When I follow the steps below, I get exception:

Exception caught in global error handler, exception message: Only Uris starting with 'ws://' or 'wss://' are supported. (Parameter 'uri'), exception stack:
   at System.Net.WebSockets.ClientWebSocket.ConnectAsync(Uri uri, CancellationToken cancellationToken)
   at Ocelot.WebSockets.Middleware.WebSocketsProxyMiddleware.Proxy(HttpContext context, String serverEndpoint)
   at Ocelot.WebSockets.Middleware.WebSocketsProxyMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.DownstreamUrlCreator.Middleware.DownstreamUrlCreatorMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.LoadBalancer.Middleware.LoadBalancingMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.Request.Middleware.DownstreamRequestInitialiserMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.Multiplexer.MultiplexingMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.DownstreamRouteFinder.Middleware.DownstreamRouteFinderMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Builder.Extensions.MapWhenMiddleware.Invoke(HttpContext context)
   at Ocelot.Errors.Middleware.ExceptionHandlerMiddleware.Invoke(HttpContext httpContext) RequestId: 6fa0fd88-82ec-4ae4-95ed-29f1b1aefd14 {"SourceContext":"Ocelot.Errors.Middleware.ExceptionHandlerMiddleware","RequestId":"80000026-0006-ff00-b63f-84710c7967bb","RequestPath":"/tieto/collection/commsvc/connectcommunicationhub","CorrelationId":"6fa0fd88-82ec-4ae4-95ed-29f1b1aefd14"}
System.ArgumentException: Only Uris starting with 'ws://' or 'wss://' are supported. (Parameter 'uri')
   at System.Net.WebSockets.ClientWebSocket.ConnectAsync(Uri uri, CancellationToken cancellationToken)
   at Ocelot.WebSockets.Middleware.WebSocketsProxyMiddleware.Proxy(HttpContext context, String serverEndpoint)
   at Ocelot.WebSockets.Middleware.WebSocketsProxyMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.DownstreamUrlCreator.Middleware.DownstreamUrlCreatorMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.LoadBalancer.Middleware.LoadBalancingMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.Request.Middleware.DownstreamRequestInitialiserMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.Multiplexer.MultiplexingMiddleware.Invoke(HttpContext httpContext)
   at Ocelot.DownstreamRouteFinder.Middleware.DownstreamRouteFinderMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Builder.Extensions.MapWhenMiddleware.Invoke(HttpContext context)
   at Ocelot.Errors.Middleware.ExceptionHandlerMiddleware.Invoke(HttpContext httpContext)

Steps to Reproduce the Problem

  1. Create a typical simple SignalR server/client application
  2. Do not specify the DownstreamScheme in the /myHub?id=<id> url route configuration
  3. Set the HttpTransportType to LongPolling on both client and server
  4. Observe that it works
  5. Set the HttpTransportType to WebSockets on both client and server
  6. Observe that now it does not work, but gives above error

In each case, following URLs should be used on downstream without changing any configuration:

LongPolling: https://localhost:1234/myHub?id=<id>
WebSockets: wss://localhost:1234/myHub?id=<id> [Logs show that Ocelot is still setting downstream to https]

If Ocelot behaves in the above (mentioned in Expected Behavior) manner, then my code will work without hassles.

@kgrosvenor
Copy link

How to fix, did you get around it?

@raman-m
Copy link
Member

raman-m commented May 27, 2023

@kgrosvenor
What to get around about?
This issue user scenario is wrong, it is invalid, and that's how it should not be used!
Details are below! 👇

@raman-m
Copy link
Member

raman-m commented May 27, 2023

@abhiphirke Hi Abhijeet!
I believe your user scenario is completely wrong.
And I am going to enlighten you...

First,
Regarding wss-protocol. Do you know this protocol is not supported by Ocelot? Yeah, I see, it is 2023 year when the people uses Web 3 protocols, and all modern browsers support latest specs of Web Sockets, HTTP2, HTTP3 protocols.
Nowadays unfortunately Ocelot does not support latest Web Sockets, SignalR protocols.
Ocelot has limited support of Web Socket old specs (ws-protocol, not encrypted). But you have to use old 15, 16 releases with old .NET releases (.NET 3.1 and .NET 5).
Probably Websockets feature in Ocelot v19 (.NET 7) is unstable. Acceptance tests are green, but you should not consider this fact as Websockets feature works corrrectly in Ocelot this 2023 year. We have a huge missing here. Much effort is needed to implement modern protocols: Web Socket, SignalR. We must understand that during these last 3-4 years Microsoft has been released .NET 5, .NET 6 and .NET 7 versions. So, .NET SDK software has changed. The System.Net namespace has been changed a lot.
Currently the community has pointed to this Web Socket problem multiple times. See:

Please, stop any experiments with wss-protocols! You can use http and https protocols only. The ws & SignalR protocols are supported partially by legacy specs.

Second,

In each case, following URLs should be used on downstream without changing any configuration:
LongPolling: https://localhost:1234/myHub?id=
WebSockets: wss://localhost:1234/myHub?id= [Logs show that Ocelot is still setting downstream to https]

That's wrong! Changing protocols requires you to provide new settings for a route. Moreover you have to enable this protocols in ASP.NET pipeline during its building in Startup.cs or Program.cs.
Another problem, you are trying to enable both protocols for the same web app endpoint (downstream service) using the same port! Practically web apps have different bindings in hosting web server, and web app must support all these protocols having all bindings enabled to the same port of the same host. Sorry, have you implemented such kind of web apps? Are you sure it is possible with ASP.NET and/or IIS servers? In theory you can bind one port to different protocols, but I am pretty sure you need to implement custom protocol which will be mix of both protocols, and/or develop/implement custom HTTP module for web server (HTTP handler/module for IIS pipeline, for example) to handle mixed-protocols traffic by one port.
So, when you specify localhost:1234 endpoint you have to choose one protocol only to bind the 1234 port to the protocol. It must be one protocol only.

[Logs show that Ocelot is still setting downstream to https]

I believe this is absolutely correct logic!

Third,

If DownstreamScheme is not specified in a Route in configuration, the upstream scheme should be used to forward request to the downstream for the URLs other than http/https.

Why do you think so?
Why must gateway fallback protocol of downstream service to some default one?
If you don't know the scheme/protocol of a service how are you gong to communicate to the endpoint of the service? In this case you will have a lot of protocol errors during communication!
Moreover does it make a sense to communicate to unknown protocol service having fallback to default protocol? In theory using statistics most of API endpoints use https protocol. But it is definitely not ws or wss protocols.
I don't know what will happen if a route config doesn't have the scheme option. Probably an exception will be generated while building URL object. I have to check this case... Anyway, we must define scheme/protocol of route.

Finally,
I do not understand your feature proposal with Upstream scheme. Having inspired by your idea, we can consider another feature like "Use default protocol from GlobalConfiguration to substitute instead of absent scheme of the DownstreamScheme option in routes".
What about this feature?

So, Abhijeet, as a software architect, could you send your feedback please, if you're still interested for sure?

@raman-m raman-m added duplicate Duplicated by another issue feature A new feature invalid Not actually an issue large effort Likely over a week of development effort medium effort Likely a few days of development effort proposal Proposal for a new functionality in Ocelot needs validation Issue has not been replicated or verified yet needs feedback Issue is waiting on feedback before acceptance waiting Waiting for answer to question or feedback from issue raiser labels May 27, 2023
@raman-m raman-m added bug Identified as a potential bug accepted Bug or feature would be accepted as a PR or is being worked on and removed duplicate Duplicated by another issue feature A new feature invalid Not actually an issue large effort Likely over a week of development effort medium effort Likely a few days of development effort proposal Proposal for a new functionality in Ocelot needs validation Issue has not been replicated or verified yet needs feedback Issue is waiting on feedback before acceptance waiting Waiting for answer to question or feedback from issue raiser labels Sep 19, 2023
@raman-m
Copy link
Member

raman-m commented Sep 19, 2023

@abhiphirke Hi Abhijeet!
Good news! Your bug-issue will be resolved by linked PR.

The issue has been accepted due to open PR #1689

@raman-m raman-m added the Websockets Ocelot feature: Websockets label Sep 19, 2023
raman-m added a commit that referenced this issue Sep 29, 2023
…SocketsProxyMiddleware (#1689)

* Update WebSocketsProxyMiddleware.cs

Fix WebSocket for SignalR

* Repalce url protocol after null check

* small refactoring

* Add error log when replacing protocol in WebSocketProxyMiddleware

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>

* Fix build

* Code review

* Fix unit test

* Refactor to remove hardcoded strings of schemes

* Define public constants

* Add unit tests

---------

Co-authored-by: raman-m <dotnet044@gmail.com>
@raman-m raman-m added merged Issue has been merged to dev and is waiting for the next release and removed accepted Bug or feature would be accepted as a PR or is being worked on labels Sep 29, 2023
@raman-m
Copy link
Member

raman-m commented Sep 29, 2023

@abhiphirke Could you test & verify the bug fix plz? You just need to update develop branch of your forked repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identified as a potential bug merged Issue has been merged to dev and is waiting for the next release Websockets Ocelot feature: Websockets
Projects
None yet
3 participants