On Tue, Mar 28, 2017 at 5:45 PM, Mark Thomas <ma...@apache.org> wrote:
> On 28/03/17 15:23, Katya Todorova wrote: > > Hi, > >> r1787662 adds Host header validation along with a fair number of unit > tests. > >> It includes a performance test which indicates - on my machine at least > >> - that the performance impact is in the noise. I'd like to see better > >> performance for full IPv6 addresses but the current code looks to be > >> acceptable. > >> The validation is not yet integrated into the request processing. My > >> primary reason for not integrating it is that it will trigger a 400 > >> response if the header is invalid and I don't want to incorrectly reject > >> valid headers. Therefore I have a request. Please try and break these > >> new parsers. > > > > > > I’ve looked at the new http host parsers and tried some test data. > > Most of the test cases have already been covered but still several > > issues popped up: > > Thanks for the additional test cases. This is exactly the sort of > feedback I was looking for. > > Would you like to get more involved in Tomcat development? If so, > turning these into a patch for the unit tests could be good place to > start. You'll need to mark the tests with @Ignore for now until the > underlying bugs are fixed. For bonus points, fix the bugs in the parser > so the tests pass. > I'll prepare a patch. Would you prefer to keep the test cases more compact or more extensive? E.g. for testing IPv6 with less components than expected we may have one test case with only 3 components (for example) or we may have many test cases - respectively with one, two, ..., seven components? The latter approach would make the test class a lot bigger but might help to catch some corner cases. Let me know what you think. Katya > Mark
