-
-
Notifications
You must be signed in to change notification settings - Fork 353
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
Changed headers handling #799
base: master
Are you sure you want to change the base?
Conversation
@gittiver could you look into this please? I have the same problem and this solves it. Although this doesn't seem like the right solution. |
I agree with @bugdea1er that this doesn't seem like the correct solution. I am quite short on time but I'd be willing to invest a little bit of it. Could you please provide a simple example program producing this error so I can try and investigate? |
I looked into this, I believe your PR highlights the weirdness that needs to be fixed. Build the headers_ string as per your patch,
The code then continues on to The problem is when there is no route rule. Crow/include/crow/http_connection.h Line 116 in 25ca2f5
This is reentering into the http parser(!) which I guess is probably not intended. Backtrace (note lines are a little different due to my hacks and because I'm using 1.2.0)
Summary, in order of call depth ... its a little hard for me to lay out nicely:
So the problem appears to stem from the path taken when we want to send a response BEFORE finishing the read. It isn't an error during reading, we really just want to stop reading any further, but wait for the async_write to complete. I guess? I'm not sure on the best fix. In the meantime, I "fixed" the problem for myself by DISABLING the async write inside do_write_general(), ie the line: Crow/include/crow/http_connection.h Line 437 in 25ca2f5
becomes: if (false && res.body.length() < res_stream_threshold_)
Then the write is able to complete before it tried to process the read any further. You still get errors in the DEBUG printouts, such as: But at least the client receives the full transmission. My client code was simple:
Note that the CORSHandler is NOT the problem, it is simply a way to add some headers that you can see will not be sent back to the client. My hack to http_connection.h, as per the commit in this PR,
Command line (failure - not enough headers returned):
Then, change Command line (success):
|
TLDR: While using Crow v1.1 our team have encountered a problem, where using a big amount of heders in OPTION request response caused those to be parsed incorrectly by a number of user-end systems, including Curl. After spending quite some time on debugging, we still couldn't fully understand the reasons for such a behavior, yet we managed to produce a fix, which we humbly ask to accept to the main Crow branch.
So. We are using Crow for backend of a web app. We've migrated to version 1.1 and encountered an unexpected and quite problematic behavior, where GnomeWeb web browser mishandled login form. After some debugging, we established the following facts :
Curl output
Expected Curl output
After some work we figured out, that not all of the headers were parsed. Also, exchange of headers positions in response resulted in the first ones being parsed correctly, while later ones were ignored. Thus we concluded, that problem was not connected to some specific header being sent incorrectly, but to the structure of response in general. We couldn't quite figure out the underlying reason for such behavior, yet the proposed change seams to fix our problem, and we believe that it can be useful to others for as long as the proper solution is not implemented.