Re: [Bug 62912] Tomcat adds a space character in the Content-Type header if this one has a ; character right after
On 24/01/2019 18:26, Christopher Schultz wrote:
> Mark,
>
> On 1/23/19 16:55, VP Brand wrote:
>> 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 --- Created
>>> attachment 36389 -->
>>> https://bz.apache.org/bugzilla/attachment.cgi?id=36389=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?
>
> This might be a rat-hole of a question, but why are we implementing
> the same logic in two different places? o.a.coyote.Response and
> o.a.catalina.connector.Response? Can we get away with implementing
> this in only one place or the other?
Maybe. Some svn archaeology would probably be required to figure out why
it was implemented this way and if that reason was still valid.
> Related: is this code-comment from o.a.coyote.Response#containsHeader
> accurate?
Yes.
Mark
>
> /**
> * Does the response contain the given header.
> *
> * Warning: This method always returns false for
> Content-Type
> * and Content-Length.
> *
> * @param name The name of the header of interest
> *
> * @return {@code true} if the response contains the header.
> */
>
> -chris
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [Bug 62912] Tomcat adds a space character in the Content-Type header if this one has a ; character right after
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Mark,
On 1/23/19 16:55, VP Brand wrote:
> 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 --- Created
>> attachment 36389 -->
>> https://bz.apache.org/bugzilla/attachment.cgi?id=36389=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?
This might be a rat-hole of a question, but why are we implementing
the same logic in two different places? o.a.coyote.Response and
o.a.catalina.connector.Response? Can we get away with implementing
this in only one place or the other?
Related: is this code-comment from o.a.coyote.Response#containsHeader
accurate?
/**
* Does the response contain the given header.
*
* Warning: This method always returns false for
Content-Type
* and Content-Length.
*
* @param name The name of the header of interest
*
* @return {@code true} if the response contains the header.
*/
- -chris
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAlxKA0AACgkQHPApP6U8
pFg1dw//Vhfz1W4MLcuZ09VIxCHPTiAFwNqqfQifkWDVmndsOmqsDX+BF3LBkhOf
50n/7Hk8X9a39NJ49v7m+FivYCN8rEAhc2DGLXY3SNlxrR54CpJnMryvZgulGC5U
2CiGSWpL3OBmyW4FggWu/RnzU1xKwvtc9kBInIXn1Zh0LsaL1C/Dx3evpdKF+YLi
apft0km2757khgYsq18D4VOs7pcBpQxogwYLcHKQ56ZWJAO8+aSUcpfuemLcVgnM
UH0es2hQF6RYWUf97e0Eu+f/yUcGx2koWLa2/+vO+EnjG4n0t/+/AmOBKZ1zcLeK
I4j6ONtlaOpRbd8px56V+ZR86ymYB5KzQkYW5HNjuzh+GdUw5L95vDtXfVCItopK
PtBi0k6GeDKuYnoKYBlhv1P0hqx7ywWwv1m2NrTOkVlrhB/DL3ocdrhNpySTSJe1
gjc6TiafLZvdlqFBvAR/HnIz6uY/AVJI3VXmhl/CPpijYub1Cf6QhjNYTUzOtyJL
h2jImB4irsal0+0jaqbCe/fo2YPtPlzkWx5ci70cRdbAY85In0sKa3ygEfGuEYhD
G6UDWng5vYTp2HX6VlPwX8zON7WUFMpCJX/oDxEeZF7NSYQu8xgBTuTpgveOVrv3
r8EAZcQc62jm9xxPM7Q7goPwSF0auCB4zHBMUxhcTgqD3h1o16w=
=qOBc
-END PGP SIGNATURE-
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [Bug 62912] Tomcat adds a space character in the Content-Type header if this one has a ; character right after
On 23/01/2019 22:42, Igal Sapir wrote: > On 1/23/2019 1:56 PM, Mark Thomas wrote: >> >> >>> 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 --- Created attachment 36389 --> https://bz.apache.org/bugzilla/attachment.cgi?id=36389=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
Re: [Bug 62912] Tomcat adds a space character in the Content-Type header if this one has a ; character right after
On 1/23/2019 1:56 PM, Mark Thomas wrote: 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 --- Created attachment 36389 --> https://bz.apache.org/bugzilla/attachment.cgi?id=36389=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. Best, Igal [1] https://github.com/apache/tomcat/blob/trunk/java/org/apache/tomcat/util/http/parser/MediaTypeCache.java#L41 - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [Bug 62912] Tomcat adds a space character in the Content-Type header if this one has a ; character right after
On 23/01/2019 21:55, VP Brand wrote: Sorry. No idea why my mail client picked that senders address. I must have hit the wrong button. Mark > 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 --- >> Created attachment 36389 >> --> https://bz.apache.org/bugzilla/attachment.cgi?id=36389=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? > > Mark > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [Bug 62912] Tomcat adds a space character in the Content-Type header if this one has a ; character right after
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 --- > Created attachment 36389 > --> https://bz.apache.org/bugzilla/attachment.cgi?id=36389=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? Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
