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

Incorrect 502 status code of downstream service when response prematurely closed with 403 #1685

Closed
gunpuz opened this issue Aug 1, 2023 · 4 comments
Assignees
Labels
bug Identified as a potential bug help wanted Not actively being worked on. If you plan to contribute, please drop a note. medium effort Likely a few days of development effort needs feedback Issue is waiting on feedback before acceptance needs validation Issue has not been replicated or verified yet question Initially seen a question could become a new feature or bug or closed ;) waiting Waiting for answer to question or feedback from issue raiser

Comments

@gunpuz
Copy link

gunpuz commented Aug 1, 2023

Expected Behavior

Ocelot service sends large file in POST → Downstream service
Returns whatever status code that Downstream service is replying.

Actual Behavior

If Downstream service does not accept the request and Ocelot is still sending the file, error occurs which results in Ocelot service status code 502, which is not what Downstream service is reporting.

Downstream service has its own Role based authorization in place
image

Downstream service always responds with 403 - Forbidden.
image

But Ocelot always responds with 502, because Downstream service RBAC returns 403 faster than request response

[11:22:01 INF] requestId: 0HMSOP9OEVPLT:0000000A, previousRequestId: no previous request id, message: 200 (OK) status code, request uri: http://eeeeee-service/eeeeee
[11:22:02 INF] requestId: 0HMSOP9OEVPM5:00000001, previousRequestId: no previous request id, message: EndpointRateLimiting is not enabled for /eeeeee{everything}
[11:22:02 INF] requestId: 0HMSOP9OEVPM5:00000001, previousRequestId: no previous request id, message: No authentication needed for /eeeeee
[11:22:02 INF] requestId: 0HMSOP9OEVPM5:00000001, previousRequestId: no previous request id, message: /eeeeee{everything} route does not require user to be authorized
[11:22:02 WRN] requestId: 0HMSOP9OEVPM5:00000001, previousRequestId: no previous request id, message: Error Code: ConnectionToDownstreamServiceError Message: Error connecting to downstream service, exception: System.Net.Http.HttpRequestException: Error while copying content to a stream.
---> System.IO.IOException: Unable to write data to the transport connection: Broken pipe.
---> System.Net.Sockets.SocketException (32): Broken pipe
--- End of inner exception stack trace ---
at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)
at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.System.Threading.Tasks.Sources.IValueTaskSource.GetResult(Int16 token)
at System.Net.Http.HttpConnection.WriteAsync(ReadOnlyMemory`1 source, Boolean async)
at System.Net.Http.HttpContent.<CopyToAsync>g__WaitAsync|56_0(ValueTask copyTask)
--- End of inner exception stack trace ---
at System.Net.Http.HttpContent.<CopyToAsync>g__WaitAsync|56_0(ValueTask copyTask)
at System.Net.Http.HttpConnection.SendRequestContentAsync(HttpRequestMessage request, HttpContentWriteStream stream, Boolean async, CancellationToken cancellationToken)
at System.Net.Http.HttpConnection.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
at System.Net.Http.HttpConnection.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
at System.Net.Http.DiagnosticsHandler.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
at Ocelot.Requester.HttpClientHttpRequester.GetResponse(HttpContext httpContext) errors found in ResponderMiddleware. Setting error response for request path:/eeeeee, request method: POST
[11:22:04 INF] requestId: 0HMSOP9OEVPLN:00000006, previousRequestId: no previous request id, message: EndpointRateLimiting is not enabled for /eeeeee{everything}

Steps to Reproduce the Problem

  1. Implement RBAC like mentioned previously in downstream service
  2. send large file which takes time via ocelot to downstream service. Use auth token (jwt for example) where role is not available.

Specifications

  • Version: 19.0.2
@raman-m raman-m changed the title Incorrect status code of downstreamservice when response prematurely closed Incorrect status code of downstream service when response prematurely closed Aug 3, 2023
@raman-m raman-m changed the title Incorrect status code of downstream service when response prematurely closed Incorrect 502 status code of downstream service when response prematurely closed with 403 Aug 3, 2023
@raman-m
Copy link
Member

raman-m commented Aug 3, 2023

Hi Guntars!
Thanks for your interest in Ocelot!

I agree, overriding of the status is not good.
This problem of status overriding is already mentioned in current issues: Search for "status code"
So, we have tons of open issues with status overriding (on developer's opinion).
Later I will find the exactly the same case when downstream service receives multi-part form data (file uploading)...

To be successful in fixing your problem, you need to collaborate more and develop your case.
You wrote a good description but it is not sufficient.


Downstream service always responds with 403 - Forbidden.
image

Why did you clear the log screenshot by white boxes?
What is authentication scheme?
What is your route config?
Could you show the config JSON plz?


But Ocelot always responds with 502, because Downstream service RBAC returns 403 faster than request response
image

Sorry, don't understand, what does this small screenshot mean?


502 status... 🆗
Let's look into current docs: Http Error Status Codes
It says: "502 if unable to connect to downstream service."
I am not sure, it requires more investigation, Ocelot loads body data into memory, and then constructs new request to downstream service. If downstream service failed (non-200) it recognizes as connection problem (502 Ocelot).
On my opinion it is correct behavior.
And I guess Ocelot doesn't provide any streaming now. But I need to check...

Possible solution

I suggest you to try the connection by OPTIONS request, first. It should return the auth-result.
If OPTIONS is OK then make the POST multi-part-form data request.
But if OPTIONS is failed then you should not make any POST requests!
And I believe OPTIONS will return you correct status which will be 403.

First step,

Could you show us that client can establish (un)successful connection please?
Any screenshots from Postman are welcome!

@raman-m raman-m added bug Identified as a potential bug help wanted Not actively being worked on. If you plan to contribute, please drop a note. question Initially seen a question could become a new feature or bug or closed ;) medium effort Likely a few days of development effort 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 Aug 3, 2023
@raman-m
Copy link
Member

raman-m commented Aug 3, 2023

Steps to Reproduce the Problem

  1. Implement RBAC like mentioned previously in downstream service
  2. send large file which takes time via ocelot to downstream service. Use auth token (jwt for example) where role is not available.

It is very abstract.
I believe it would be quite difficult to reproduce such an issue. 🤣

Could you upload your solution to GitHub please, and share a link?
That will help to reproduce the issue. It will save a lot of time for contributors.

And, do you have a draft solution to fix the problem of status overriding?
How did you solve this problem?

@gunpuz
Copy link
Author

gunpuz commented Aug 9, 2023

Hi, I have updated the issue description.

I see now what did you mean about streaming, it would be better if streaming option would be available. Because the downstream service responds quickly, but the file gets stuck in Ocelot till it is cached. So simple RBAC check could take resources instead of just returning error to client. This is maybe a seperate issue #695

image

Here is example configuration of ocelot

{
  "SwaggerKey": "eeeee",
  "UpstreamPathTemplate": "/eeeee{everything}",
  "UpstreamHttpMethod": [
    "Get",
    "Post",
    "Put",
    "Delete"
  ],
  "DownstreamPathTemplate": "/eeeee{everything}",
  "DownstreamScheme": "http",
  "DownstreamHostAndPorts": [
    {
      "Host": "localhost",
      "Port": 5002
    }
  ]
}

502 status... 🆗
Let's look into current docs: Http Error Status Codes
It says: "502 if unable to connect to downstream service."
I am not sure, it requires more investigation, Ocelot loads body data into memory, and then constructs new request to downstream service. If downstream service failed (non-200) it recognizes as connection problem (502 Ocelot).
On my opinion it is correct behavior.

Yes it could be correct response from Ocelot, but this means that downstream services cannot respond with non-200 status code, which makes harder to seperate micro services and api. This could be one of behaviours, but changing status code to downstream service should be an option. Also there is question - should 502 be only responsible for status codes 5xx from downsteream services?

@raman-m
Copy link
Member

raman-m commented Aug 9, 2023

@gunpuz commented on Aug 9:

I see now what did you mean about streaming, it would be better if streaming option would be available. Because the downstream service responds quickly, but the file gets stuck in Ocelot till it is cached. So simple RBAC check could take resources instead of just returning error to client. This is maybe a separate issue #695

I don't care about streaming feature now because it is absolutely different feature. If you wish, continue discussion in appropriate issue thread please.
Let's focus on the current issue with status codes!


Also there is question - should 502 be only responsible for status codes 5xx from downstream services?

No, not for all. Current error mapper produces 502 in case of any connection errors: ErrorsToHttpStatusCodeMapper
You can have a deeper analysis of source code.


Yes it could be correct response from Ocelot, but this means that downstream services cannot respond with non-200 status code, which makes harder to separate micro services and api. This could be one of behaviours, but changing status code to downstream service should be an option.

Let's focus on the problem of status changing...
As far as I understood, you need to have original status code in response.
Current Ocelot design of error processing is based on mapping errors to internal Ocelot codes: ErrorsToHttpStatusCodeMapper, Http Error Status Codes.
That means response has no original status code. I'm not sure, need to check it.
There could be only one way to get original status, via Headers Transformation feature. But I don't see appropriate Placeholders for status code. And it can be a fix: to introduce appropriate placeholder for status code, and add to response as a custom header.
So, I see 2 possible solutions:

  • Introduce new placeholder for original status code of response
  • Redesign responder to hardcode a header or just re-add the header to a response, and/or introduce Ocelot native header of status code, for all responses

What solution is the best for you?
But I'm pretty sure, it won't be difficult to implement them both...

@ThreeMammals ThreeMammals deleted a comment from gunpuz Nov 9, 2023
@raman-m raman-m closed this as not planned Won't fix, can't repro, duplicate, stale Nov 9, 2023
@ThreeMammals ThreeMammals locked and limited conversation to collaborators Nov 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Identified as a potential bug help wanted Not actively being worked on. If you plan to contribute, please drop a note. medium effort Likely a few days of development effort needs feedback Issue is waiting on feedback before acceptance needs validation Issue has not been replicated or verified yet question Initially seen a question could become a new feature or bug or closed ;) waiting Waiting for answer to question or feedback from issue raiser
Projects
None yet
Development

No branches or pull requests

2 participants