Issue #132: Middleware composition regression causes contant 406 Not Acceptable with push_ssl on
Reported by: | saaj |
State: | resolved |
Created on: | 2015-05-07 16:55 |
Updated on: | 2015-06-19 16:30 |
Description
This commit by @kiilerix breaks the composition. I know nothing about Pylons' middleware but through wasting half a day in a WTF mode, I know that this is broken (HttpsFixup
is never called on VCS opertaions):
# Enable https redirects based on HTTP_X_URL_SCHEME set by proxy if any(asbool(config.get(x)) for x in ['https_fixup', 'force_https', 'use_htsts']): app = HttpsFixup(app, config) # we want our low level middleware to get to the request ASAP. We don't # need any pylons stack middleware in them - especially no StatusCodeRedirect buffering app = SimpleHg(app, config) app = SimpleGit(app, config)
And this does work (the order it was in RhodeCode):
# we want our low level middleware to get to the request ASAP. We don't # need any pylons stack middleware in them - especially no StatusCodeRedirect buffering app = SimpleHg(app, config) app = SimpleGit(app, config) # Enable https redirects based on HTTP_X_URL_SCHEME set by proxy if any(asbool(config.get(x)) for x in ['https_fixup', 'force_https', 'use_htsts']): app = HttpsFixup(app, config)
What's really alerting about it is that this regression in important functionality has infiltrated though the test suite. Either tests are not good enough or the commiters don't examine them.
Attachments
Comments
Comment by saaj, on 2015-05-07 16:56
Comment by Mads Kiilerich, on 2015-05-07 17:03
Yes, the test coverage is bad. Contributions are welcome.
What problem do you experience?
Comment by saaj, on 2015-05-07 17:09
Don't the title and the body say what is the problem? Well, I can repeat -- with composition in the first snippet when in UI Require SSL for vcs operations is checked, HttpsFixup
is never called thus BaseVCSController._check_ssl
always returns False
and SimpleHg
always returns HTTPNotAcceptable
. Restore correct composition.
Comment by Mads Kiilerich, on 2015-05-07 17:43
Please don't be a jerk when reporting issues.
Comment by saaj, on 2015-05-12 11:50
It's too much a gratitude for hours it took me to debug your irresponsible changes that add nothing to RhodeCode but breaking it. Please don't be an imbecile -- educate yourself with RhodeCode codebase, don't ask stupid generic questions on an issue (and fix) specifically described in terms of codebase. And lastly be responsible! Don't introduce changes into a code you don't understand, and at very least test your changes!
As a note to future readers interested in RhodeCode vs Kallithea. Kallithea as of 0.2.1 has no advantage over RhodeCode 1.7.1 (which is still available on the CheeseShop). It just works and is fine for a trusted environment and is the same GPLv3. Though, Python code of RhodeCode 2.x is still GPLv3, and if you aren't fine with its restrictions and don't want to buy it, patch patch the licensing module. The benefit is properly maintained product, not somebody's toy with poor competence and manners.
Unwatched. I stay with RhodeCode 1.7.1. Kallithea with several regressions (this is not the only) and the attitude lost my interest.
Comment by Sean Farley, on 2015-05-12 18:26
The sad part is that your attitude has cost you an easy path to get this fixed.
Comment by Thomas De Schampheleire, on 2015-05-12 20:56
@saaj When an issue is reported to an open-source project like Kallithea, the assumption is that both the submitter and the project contributors have a shared interest in understanding and solving the problem. Additional questions by project developers are not asked to to bother you but to better understand the issue.
The 'What problem do you experience' question asked by @kiilerix may have been redundant given the issue title, but his apparent overlooking of the information in the title is not so abnormal: it is customary that the description contains all relevant information, in particular when they are crucial in understanding the problem. Moreover, while your investigation of the problem was clear in terms of code difference between a working and non-working case, there were little details about the environment in which you deploy Kallithea (other than push_ssl being enabled) and most importantly which sequence of actions led to the 406 error.
Regardless, your last reaction (132.html#comment-18046728) was really uncalled for. Terms like 'too much a gratitude', 'irresponsible', 'imbecile', 'stupid generic questions' and 'poor competence and manners' can possible be used when interacting with a commercial vendor, but definitely not when interacting with an open-source community like Kallithea's. Asserting that @kiilerix does not understand the codebase and should educate himself is also not necessary: his contributions date back to end of 2012 when the project was still RhodeCode, and his understanding of the code is undoubtedly very good.
Nobody is forcing anyone to use Kallithea rather than RhodeCode. If you're happy with RhodeCode and do not see the advantages of community-based open-source development, then by all means continue to use it.
Finally, the issue you reported is supposed to be fixed with following commit, soon to be pushed to Kallithea mainline: https://bitbucket.org/Unity-Technologies/kallithea/commits/58aedf5be5b36628f7bbb36a046f19f17a279701 Feel free to test and report back if it works for you.
Comment by saaj, on 2015-05-13 10:09
@patrickdepinguin First, thank you for your attempt to clarify the situation.
When an issue is reported to an open-source project like Kallithea, the assumption is that both the submitter and the project contributors have a shared interest in understanding and solving the problem. Additional questions by project developers are not asked to to bother you but to better understand the issue.
Agreed. Specific and reasonable follow-ups are perfectly fine. In contrary the question, draws impression of either lack of knowledge of the codebase, or reluctance to putting a little effort in reading and comprehending what was reported. So to me it looks like the contributor didn't share the interest.
... Moreover, while your investigation of the problem was clear in terms of code difference between a working and non-working case, there were little details about the environment in which you deploy Kallithea (other than push_ssl being enabled) and most importantly which sequence of actions led to the 406 error.
If it was relevant I would have told. There was no other relevant configuration but push_ssl
.
Regardless, your last reaction (132.html#comment-18046728) was really uncalled for. Terms like 'too much a gratitude', 'irresponsible', 'imbecile', 'stupid generic questions' and 'poor competence and manners' can possible be used when interacting with a commercial vendor, but definitely not when interacting with an open-source community like Kallithea's. Asserting that kiilerix does not understand the codebase and should educate himself is also not necessary: his contributions date back to end of 2012 when the project was still RhodeCode, and his understanding of the code is undoubtedly very good.
To me it's the very first time when I was spoken this way in a open source community. I want to emphasize -- not on a street, not talking to a buddy, but in a open source community! When I submit a bugreport to a project, I'm talking as engineer to engineer, or as developer to developer. Nursing somebody's lack of effort is never my point. Especially when I'm pointing to somebody's mistake that costed me several hours of debugging, I expect some effort and a little empathy. When I get none, and moreover told who I am and who shouldn't I be, the reaction is obvious. I can reply symmetrically, but just for first and last time.
But please, Thomas, regardless of emotional part, in technical part lets call spade a spade. Generally, when a developer dares to change a code he doesn't understand, and after doesn't test his changes, he is irresponsible. It is a matter of fact, and I don't see the need to argue about it. When the lack of understanding is revealed the developer should educate oneself and fill the gap (Pylons middleware composition). So as long as I was forced into the street-like conversation I preferred to avoid any euphemisms.
Nobody is forcing anyone to use Kallithea rather than RhodeCode. If you're happy with RhodeCode and do not see the advantages of community-based open-source development, then by all means continue to use it.
I know, I just wanted to illustrate cause and effect.
Finally, the issue you reported is supposed to be fixed with following commit, soon to be pushed to Kallithea mainline: https://bitbucket.org/Unity-Technologies/kallithea/commits/58aedf5be5b36628f7bbb36a046f19f17a279701 Feel free to test and report back if it works for you.
I don't take my words back. I hope you can understand that I was discouraged and I won't participate at least in short-term. Moreover I wiped Kallithea installation and migrated RhodeCode instance to the new server. So all I can say is that if you have restored original composition is should be okay. Also I would suggest using Docker for manual testing.
Comment by Mads Kiilerich, on 2015-06-19 16:30
resolved on stable branch