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
Dont set content-length header on streamed response. Fixes #13991 #2582
Conversation
@@ -505,6 +511,8 @@ bool QgsHttpRequestHandler::startGetFeatureResponse( QByteArray* ba, const QStri | |||
format = "text/xml"; | |||
|
|||
setInfoFormat( format ); | |||
startStreamingResponse(); |
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.
Are you setting mStreamingResponse
for all requests?
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.
@elpaso All "GetFeature"-requests. As I understand the code the response for GetFeature-requests are always streamed. I also think it is the only request type to get streamed response.
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.
Hi @AsgerPetersen you didn't need to create startStreamingResponse.
To fix it, just add sendHeaders(); just after line 507 setInfoFormat( format ); and it's worked.
Can I close your Pull-Request and push my fix ?
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.
@AsgerPetersen yeah, sorry I just had a quick look and it seemed to me that you added startStreamingResponse();
inside sendResponse()
that means for ALL request including WMS etc.
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.
@rldhont Thanks for fixing this issue!
To explain my take on the code sendHeaders()
was also my first take on the issue, but I thought it would be nice to make it more explicit when you want to start a streaming response as opposed to a standard response. The mStreamingResponse
could also be used to protect against programming errors in the form of multiple calls to sendResponse()
outside a streaming response.
dd116f9
to
3087dc0
Compare
Thanks to @AsgerPetersen to help fixing #13991 closes #2582
Thanks to @AsgerPetersen to help fixing #13991 closes #2582
Thanks to @AsgerPetersen to help fixing #13991 closes #2582
No description provided.