On 23/01/2019 22:42, Igal Sapir wrote: > On 1/23/2019 1:56 PM, Mark Thomas wrote: >> <snip/> >> >>> On 23/01/2019 21:48, bugzi...@apache.org wrote: >>>> https://bz.apache.org/bugzilla/show_bug.cgi?id=62912 >>>> >>>> --- Comment #14 from Mark Thomas <ma...@apache.org> --- >>>> Created attachment 36389 >>>> --> >>>> https://bz.apache.org/bugzilla/attachment.cgi?id=36389&action=edit >>>> Tomcat 9 patch to retain app provided content-type >>>> >>>> The application provided content-type is only retained if no charset is >>>> present. >>> Thoughts on applying this patch to Tomcat 9? >>> >>> Pros: It allows apps to work with both user agents that require spaces >>> and user agents that require no spaces >>> >>> Cons: The switch to passing through the app provided value may break >>> some user agents and an app change would be required to fix it. >>> >>> The risk looks to be very low but so is the scale of the problem this >>> fixes. One bug report in a little over 8 years suggests this is an issue >>> for a small minority of users. >>> >>> If anything, I am leaning towards applying the patch. Thoughts? > > It looks safe to me. > > While reviewing the code I noticed the use of a String[] instead of a > simple class [1], which would make the code much more readable and more > maintainable. I have to admit that I reviewed it online on GitHub, it > is probably not as bad in an IDE. > > Would it make sense for me to refactor it? The new class can be > internal, possibly package private.
I can't remember why I did it that way. It might have been related to GC/performance overhead. It would be worth checking the impact of any change. It might just have been that a String array was less code. The class will need to be public since it will be used by multiple packages. OK, we could use an interface but really does start to look like over engineering. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
