-
Notifications
You must be signed in to change notification settings - Fork 109
Fix issue with 411 Length Required HTTP error #12371
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
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
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 the thorough investigation, i agree with the solution proposed. we have to live with what we have. thanks valentin
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.
Even though this might be a working solution, I wonder why APS drops an HTTP header provided by the client. If there is no strong reason for that, then the correct way to fix it would be updating the APS code to stop doing that.
@amaltaro , it is not issue with APS but it is how GoLang team interpret HTTP standard. The standard does not specify if this header is mandatory or not in this particular situation, and therefore it is up to dev teams to treat it as they think the best. Because of that some frameworks discard it, others requests its explicitly. Bottom line, nothing I can do within APS itself. If you are not satisfied with it, we'll need to raise issue with GoLang development team. |
I see. I thought it had been an implementation decision in APS. If it is in the GoLang HTTP library, then yes, I am afraid that there is no other place to make this change. What we can do though is to upgrade the CherryPy version in WMCore - which is already in the plans before we freeze the code. So I'd suggest to come back to this in a week or so and test with a new version of CherryPy. Or perhaps we could even pip upgrade CherryPy in our reqmgr2 container and see if it will work out of the box. |
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 @vkuznet for sharing what you have found, and for the fix you have provided.
That's approved from my side.
Regarding @amaltaro 's comment, I won't keep this hanging until we update the CherryPy version we use, as the latter is not part of our Q2 priority plan. I would rather suggest merging this PR and coming back to this topic with a new issue once we have a working transition to CherryPy.
After further R&D I found that the 411 HTTP error code literally comes from WM stack rather the version of CherryPy. First, I confirmed that using Flask Python framework to proof that using
I can perform the following curl call to it and it passes just fine:
So, the Then, I installed two CherryPy versions, one we are using and another using latest release. For testing I used the following web server:
With this server we get the following
But in a server log I see the caught exception:
The internal CherryPy server error (if exception is not caught), is 500 Server error rather the 411 one. Therefore, using stock CherryPy server does not lead to 411 HTTP error when we send Instead, the WM web framework somewhere sets constrain that DELETE (and most likely any othe HTTP ) method must be accomplished with Therefore, my conclusion remains the same:
For instance, the 411 error occurs from Therefore, the easiest way to fix reported issue is what I provided in this PR, i.e. before internal web server will process incoming HTTP request we set up I hope this further clarifies the underlying issue. |
@vkuznet may you squash the commits, and we eventually merge them? |
It was only one commit, but I took liberty to improve docstring for proposed function to reflect my later findings. |
Jenkins results:
|
I saw that new unit test failed
But it is unstable or rather based from its log I see it is not unrelated:
|
Thanks @vkuznet, I'm going to retrigger Jenkins to check that it's a temporary issue |
test this please |
Jenkins results:
|
I'm merging the PR |
Fixes #12348
Status
ready
Description
We observed in reqmgr2 logs the following:
This messages appears upon reqmgr2-tasks HTTP client calls
/reqmgr2/data/transferinfo
end-point which is served by CherryPy server. IfDELETE
request does not contain payload body the CherryPy complains with 411 error code. This PR provides a fix to setupContent-Length
HTTP header forDELETE
method within CherryPy server before processing this request. For more details please see #12348 (comment)Notes:
through comprehensive R&D I found the following:
Content-Length
HTTP header if body is nil. At the same time it preserve ContentLength within HTTP request itself (not as separate HTTP header).Based on my findings I opted to use solution to modify CherryPy server with proper configuration to gracefully handle this case.
Is it backward compatible (if not, which system it affects?)
YES
Related PRs
External dependencies / deployment changes